Repository: dotenv-linter/dotenv-linter
Pull request #328 (2020-10-19 20:55:25)

Fix cyclic symbolic link recursion
When in recursive mode: do not follow symbolic links within target directories. See #326.

 - Add check in crate::get_file_paths to only get subdirectories if
   they are not symlinks.

 - Add test with a cyclic symlink.

 - Add symlink creation function for unix and windows to use in test.

<!--
Thank you for your pull request. Please provide a description above and review
the requirements below.

Bug fixes and new features should include tests.
-->

<!-- _Please make sure to review and check all of these items:_ -->

#### ✔ Checklist:
<!-- Remove items that do not apply. For completed items, change [ ] to [x]. -->

- [x] This PR has been added to [CHANGELOG.md](https://github.com/dotenv-linter/dotenv-linter/blob/master/CHANGELOG.md) (at the top of the list);
- [x] Tests for the changes have been added (for bug fixes / features);
- [x] Docs have been added / updated (for bug fixes / features).

<!-- _NOTE: these things are not required to open a PR and can be done afterwards / while the PR is open._ -->


codecov-io: # [Codecov](https://codecov.io/gh/dotenv-linter/dotenv-linter/pull/328?src=pr&el=h1) Report
> Merging [#328](https://codecov.io/gh/dotenv-linter/dotenv-linter/pull/328?src=pr&el=desc) into [master](https://codecov.io/gh/dotenv-linter/dotenv-linter/commit/fdc67d77a96cd29983c8d0a8f65657f2686f55a5?el=desc) will **increase** coverage by `0.00%`.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/dotenv-linter/dotenv-linter/pull/328/graphs/tree.svg?width=650&height=150&src=pr&token=wQybePfHI1)](https://codecov.io/gh/dotenv-linter/dotenv-linter/pull/328?src=pr&el=tree)

```diff
@@           Coverage Diff           @@
##           master     #328   +/-   ##
=======================================
  Coverage   96.98%   96.99%           
=======================================
  Files          32       32           
  Lines        2292     2295    +3     
=======================================
+ Hits         2223     2226    +3     
  Misses         69       69           
```


| [Impacted Files](https://codecov.io/gh/dotenv-linter/dotenv-linter/pull/328?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [src/lib.rs](https://codecov.io/gh/dotenv-linter/dotenv-linter/pull/328/diff?src=pr&el=tree#diff-c3JjL2xpYi5ycw==) | `98.57% <100.00%> (+0.06%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/dotenv-linter/dotenv-linter/pull/328?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/dotenv-linter/dotenv-linter/pull/328?src=pr&el=footer). Last update [fdc67d7...263cc7e](https://codecov.io/gh/dotenv-linter/dotenv-linter/pull/328?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 2244aab..90e9fde 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -10,6 +10,7 @@ ### 🚀 Added
 - Add default exclude list [#324](https://github.com/dotenv-linter/dotenv-linter/pull/324) ([@ametalon](https://github.com/ametalon))
 
 ### 🔧 Changed
+- Fix bug where symlinks would cycle in recursive mode [#328](https://github.com/dotenv-linter/dotenv-linter/pull/328) ([@sonro](https://github.com/sonro))
 - Fix linter rechecking files if they were listed more than once [#327](https://github.com/dotenv-linter/dotenv-linter/pull/327) ([@Aashu23](https://github.com/Aashu23))
 - Added docker build step to the CI pipeline [#322](https://github.com/dotenv-linter/dotenv-linter/pull/322) ([@JoeAmedeo](https://github.com/JoeAmedeo))
 - Change soon deprecated `set-env` action [#320](https://github.com/dotenv-linter/dotenv-linter/pull/320) ([@marcodenisi](https://github.com/marcodenisi))
diff --git a/src/lib.rs b/src/lib.rs
index f60779f..1870c4a 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -86,7 +86,10 @@ fn get_file_paths(
             read_dir
                 .filter_map(|e| e.ok())
                 .map(|e| e.path())
-                .filter(|path| FileEntry::is_env_file(path) || (is_recursive && path.is_dir()))
+                .filter(|path| {
+                    FileEntry::is_env_file(path)
+                        || (is_recursive && path.is_dir() && path.read_link().is_err())
+                })
                 .collect()
         })
         .flat_map(|dir_entries| get_file_paths(dir_entries, excludes, is_recursive))


Test patch:
diff --git a/tests/common/mod.rs b/tests/common/mod.rs
index 8abf23f..7dca2dc 100644
--- a/tests/common/mod.rs
+++ b/tests/common/mod.rs
@@ -1,6 +1,7 @@
 mod output;
 mod test_dir;
 mod test_file;
+mod test_link;
 
 pub use output::*;
 pub use test_dir::TestDir;
diff --git a/tests/common/test_dir.rs b/tests/common/test_dir.rs
index 1e58d37..daac9ea 100644
--- a/tests/common/test_dir.rs
+++ b/tests/common/test_dir.rs
@@ -6,6 +6,7 @@
 use dunce::canonicalize;
 
 use crate::common::test_file::TestFile;
+use crate::common::test_link::create_test_symlink;
 #[cfg(not(windows))]
 use std::fs::canonicalize;
 
@@ -50,6 +51,12 @@ pub fn create_testfile(&self, name: &str, contents: &str) -> TestFile {
         TestFile::new(&self.current_dir, name, contents)
     }
 
+    /// Create a new TestLink to a TestDir within the TestDir
+    pub fn create_symlink(&self, source_test_dir: &Self, name: &str) {
+        let dest = &self.current_dir.path().join(name);
+        create_test_symlink(&source_test_dir.current_dir, dest);
+    }
+
     /// Get full path of TestDir as a &str
     pub fn as_str(&self) -> &str {
         self.current_dir
diff --git a/tests/common/test_link.rs b/tests/common/test_link.rs
new file mode 100644
index 0000000..aed8ccf
--- /dev/null
+++ b/tests/common/test_link.rs
@@ -0,0 +1,13 @@
+use std::os;
+use std::path::Path;
+use tempfile::TempDir;
+
+/// Create a new symlink within a TempDir
+#[cfg(not(windows))]
+pub fn create_test_symlink(source_test_dir: &TempDir, dest: &Path) {
+    os::unix::fs::symlink(source_test_dir, dest).expect("create symlink");
+}
+#[cfg(windows)]
+pub fn create_test_symlink(source_test_dir: &TempDir, dest: &Path) {
+    os::windows::fs::symlink_dir(source_test_dir, dest).expect("create symlink");
+}
diff --git a/tests/flags/recursive.rs b/tests/flags/recursive.rs
index f30ea5a..0004a00 100644
--- a/tests/flags/recursive.rs
+++ b/tests/flags/recursive.rs
@@ -80,3 +80,23 @@ fn checks_recursive_with_exclude_subdir() {
 
     test_dir.test_command_fail_with_args(args, expected_output);
 }
+
+#[test]
+fn checks_nofollow_subdir_symlinks() {
+    let test_dir = TestDir::new();
+    let test_subdir = test_dir.subdir();
+    let testfile = test_subdir.create_testfile(".incorrect.env", "1BAR=\n");
+    // create a symbolic link to its containing directory
+    test_subdir.create_symlink(&test_subdir, "symlink");
+
+    let args = &["-r"];
+    let expected_output = format!(
+        "{}:1 LeadingCharacter: Invalid leading character detected\n\nFound 1 problem\n",
+        Path::new(&test_dir.relative_path(&test_subdir))
+            .join(testfile.shortname_as_str())
+            .to_str()
+            .expect("multi-platform path to test .env file")
+    );
+
+    test_dir.test_command_fail_with_args(args, expected_output);
+}