diff --git a/src/shell-integration/helpers.js b/src/shell-integration/helpers.js index 7808c7e..7efface 100644 --- a/src/shell-integration/helpers.js +++ b/src/shell-integration/helpers.js @@ -28,11 +28,35 @@ export function removeLinesMatchingPattern(filePath, pattern) { } const fileContent = fs.readFileSync(filePath, "utf-8"); - const lines = fileContent.split(os.EOL); - const updatedLines = lines.filter((line) => !pattern.test(line)); + const lines = fileContent.split(/[\r\n\u2028\u2029]+/); + const updatedLines = lines.filter((line) => !shouldRemoveLine(line, pattern)); fs.writeFileSync(filePath, updatedLines.join(os.EOL), "utf-8"); } +const maxLineLength = 100; +function shouldRemoveLine(line, pattern) { + const isPatternMatch = pattern.test(line); + + if (!isPatternMatch) { + return false; + } + + if (line.length > maxLineLength) { + // safe-chain only adds lines shorter than maxLineLength + // so if the line is longer, it must be from a different + // source and could be dangerous to remove + return false; + } + + if (line.includes("\n") || line.includes("\r") || line.includes("\u2028") || line.includes("\u2029")) { + // If the line contains newlines, something has gone wrong in splitting + // \u2028 and \u2029 are Unicode line separator characters (line and paragraph separators) + return false; + } + + return true; +} + export function addLineToFile(filePath, line) { if (!fs.existsSync(filePath)) { fs.writeFileSync(filePath, "", "utf-8"); diff --git a/src/shell-integration/helpers.spec.js b/src/shell-integration/helpers.spec.js new file mode 100644 index 0000000..264cc90 --- /dev/null +++ b/src/shell-integration/helpers.spec.js @@ -0,0 +1,113 @@ +import { describe, it, beforeEach, afterEach, mock } from "node:test"; +import assert from "node:assert"; +import { tmpdir } from "node:os"; +import fs from "node:fs"; +import path from "path"; + +describe("removeLinesMatchingPatternTests", () => { + let testFile; + + beforeEach(() => { + // Create temporary test file + testFile = path.join(tmpdir(), `test-helpers-${Date.now()}.txt`); + + // Mock the os module to override EOL + mock.module("node:os", { + namedExports: { + EOL: "\r\n", // Simulate Windows line endings + tmpdir: tmpdir, + platform: () => "linux" + } + }); + }); + + afterEach(() => { + // Clean up test files + if (fs.existsSync(testFile)) { + fs.unlinkSync(testFile); + } + + // Reset mocks + mock.reset(); + }); + + + it("should handle mixed line endings without wiping entire file", async () => { + // Import helpers after setting up the mock + const { removeLinesMatchingPattern } = await import("./helpers.js"); + + // Create a file with Unix line endings but os.EOL expects Windows + const fileContent = [ + "# keep this line", + "alias npm='remove-this'", + "# keep this line too", + "alias yarn='remove-this-too'", + "# final line to keep" + ].join("\n"); // File has Unix line endings + + fs.writeFileSync(testFile, fileContent, "utf-8"); + + // Try to remove lines containing 'alias' + const pattern = /alias.*=/; + removeLinesMatchingPattern(testFile, pattern); + + const result = fs.readFileSync(testFile, "utf-8"); + + // This test will fail because the function splits on '\r\n' but file uses '\n' + // So it treats the entire content as one line and if any part matches, removes everything + assert.ok(result.includes("keep this line"), "Should preserve non-matching lines"); + assert.ok(result.includes("final line to keep"), "Should preserve final line"); + }); + + it("should handle mixed line endings with short matching content", async () => { + // Import helpers after setting up the mock + const { removeLinesMatchingPattern } = await import("./helpers.js"); + + // Create a file with Unix line endings, but make the entire content short + // to bypass the maxLineLength protection + const fileContent = [ + "# keep1", + "alias x=y", // Short alias line that should be removed + "# keep2" + ].join("\n"); // File has Unix line endings, total length < 100 chars + + fs.writeFileSync(testFile, fileContent, "utf-8"); + + // Try to remove lines containing 'alias' + const pattern = /alias/; + removeLinesMatchingPattern(testFile, pattern); + + const result = fs.readFileSync(testFile, "utf-8"); + + // This should now be protected by the newline detection + assert.ok(result.includes("keep1"), "Should preserve first line"); + assert.ok(result.includes("keep2"), "Should preserve third line"); + }); + + it("should handle Unicode line separators that bypass newline detection", async () => { + // Import helpers after setting up the mock + const { removeLinesMatchingPattern } = await import("./helpers.js"); + + // Use Unicode line separator (U+2028) and paragraph separator (U+2029) + // These are considered line breaks but aren't \n or \r + const fileContent = [ + "keep this", + "alias test=value", + "keep that" + ].join("\u2028"); // Unicode line separator + + fs.writeFileSync(testFile, fileContent, "utf-8"); + + // Try to remove lines containing 'alias' + const pattern = /alias/; + removeLinesMatchingPattern(testFile, pattern); + + const result = fs.readFileSync(testFile, "utf-8"); + + // This could still wipe everything if split() treats it as one line + // but the content doesn't contain \n or \r so passes the newline check + assert.ok(result.includes("keep this"), "Should preserve first part"); + assert.ok(result.includes("keep that"), "Should preserve last part"); + }); + +});