fix: create valid Standard JSON to verify for projects with symlinks (#…

…35)

Currently, when a project includes symbolic links to Solidity files,
`Project::standard_json_input` returns a Standard JSON Input struct that
is unusable for contract verification on block explorers. `solc` cannot
compile these contracts based on the json, leading to verification
failures. This pull request addresses this issue.

I encountered this problem when using the `forge verify-contract`
command, which internally uses this function. While `forge build`
completes successfully, the verification command fails, indicating that
the block explorer cannot find a source file.

This problem occurs in projects that contain symlinked Solidity files
and when these files are imported from other files. My project,
structured as a monorepo, uses external libraries installed via pnpm.
These libraries, accessible from the `./node_modules/` directory, are
set up with remappings in my Foundry project. However, directories under
`./node_modules/` are symlinks pointing to other locations, leading to
this issue.

## Reproduction

I added a test named `can_create_standard_json_input_with_symlink` to
demonstrate the issue within this repository. Also, the error can be
reproduced using the `forge verify-contract` command and steps below:

Environment: macOS (Apple silicon), forge 0.2.0 (`c312c0d`
2023-12-22T00:20:29.297186000Z)

```console
$ mkdir repro && cd repro

$ mkdir -p project/src project/node_modules dependency

$ cat << EOF > project/src/A.sol
// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.23;
import "@dependency/B.sol";
contract A is B {}
EOF

$ cat << EOF > dependency/B.sol
// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.23;
import "./C.sol";
contract B is C {}
EOF

$ cat << EOF > dependency/C.sol
// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.23;
contract C {}
EOF

$ cat << EOF > project/foundry.toml
[profile.default]
remappings = ["@dependency/=node_modules/dependency/"]
allow_paths = ["../dependency/"]
EOF

# Create a symbolic link
$ cd project/node_modules
$ ln -s ../../dependency dependency
$ cd ../..

# Display the file structure
$ tree
.
├── dependency
│   ├── B.sol
│   └── C.sol
└── project
    ├── foundry.toml
    ├── node_modules
    │   └── dependency -> ../../dependency
    └── src
        └── A.sol

# `build` succeeds
$ cd project
$ forge build

# `verify-contract` generates an unintended json (C.sol has a host absolute path name)
$ forge verify-contract --show-standard-json-input 0x0000000000000000000000000000000000000000 A | jq . > A.json
$ cat A.json
{
  "language": "Solidity",
  "sources": {
    "src/A.sol": {
      "content": "// SPDX-License-Identifier: UNLICENSED\npragma solidity ^0.8.23;\nimport \"@dependency/B.sol\";\ncontract A is B {}\n"
    },
    "node_modules/dependency/B.sol": {
      "content": "// SPDX-License-Identifier: UNLICENSED\npragma solidity ^0.8.23;\nimport \"./C.sol\";\ncontract B is C {}\n"
    },
    "/Users/t/bench/repro/dependency/C.sol": {
      "content": "// SPDX-License-Identifier: UNLICENSED\npragma solidity ^0.8.23;\ncontract C {}\n"
    }
  },
  "settings": {
    "remappings": [
      "@dependency/=node_modules/dependency/",
      "dependency/=node_modules/dependency/"
    ],
    "optimizer": {
      "enabled": true,
      "runs": 200
    },
    "metadata": {
      "useLiteralContent": false,
      "bytecodeHash": "ipfs",
      "appendCBOR": true
    },
    "outputSelection": {
      "*": {
        "": [
          "ast"
        ],
        "*": [
          "abi",
          "evm.bytecode",
          "evm.deployedBytecode",
          "evm.methodIdentifiers",
          "metadata"
        ]
      }
    },
    "evmVersion": "paris",
    "libraries": {}
  }
}

# `solc` cannot compile using the json
$ solc --standard-json --no-import-callback A.json | jq .errors
[
  {
    "component": "general",
    "errorCode": "6275",
    "formattedMessage": "ParserError: Source \"node_modules/dependency/C.sol\" not found: No import callback.\n --> node_modules/dependency/B.sol:3:1:\n  |\n3 | import \"./C.sol\";\n  | ^^^^^^^^^^^^^^^^^\n\n",
    "message": "Source \"node_modules/dependency/C.sol\" not found: No import callback.",
    "severity": "error",
    "sourceLocation": {
      "end": 81,
      "file": "node_modules/dependency/B.sol",
      "start": 64
    },
    "type": "ParserError"
  }
]
```

Manually editing the host filesystem's absolute path as shown below
allows the `solc` command to succeed.

```diff
   "node_modules/dependency/B.sol": {
     "content": "// SPDX-License-Identifier: UNLICENSED\npragma solidity ^0.8.23;\nimport \"./C.sol\";\ncontract B is C {}\n"
   },
-  "/Users/t/bench/repro/dependency/C.sol": {
+  "node_modules/dependency/C.sol": {
     "content": "// SPDX-License-Identifier: UNLICENSED\npragma solidity ^0.8.23;\ncontract C {}\n"
   }
 },
```

## Cause

The issue arises because import paths in Solidity files are processed by
the
[`std::fs::canonicalize`](https://doc.rust-lang.org/std/fs/fn.canonicalize.html)
function. This function resolves symlinks and normalizes paths. When
symlinks are resolved, it results in `solc` being unable to locate the
corresponding source paths in the json, as it relies on Solidity import
statements. Therefore, symlinks should not be resolved here. The paths
should be maintained as specified in the Solidity files, except for
basic normalization.

## Solution

To address this, I implemented an import path normalization function and
replaced the canonicalization function where necessary. Based on [the
Solidity documentation
page](https://docs.soliditylang.org/en/v0.8.23/path-resolution.html),
this function resolves `.` and `..` segments/components without
resolving symlinks.

The Standard JSON's source paths, for verification purposes, should be
based on the project root. This allows the compiler to find sources
within the json. The conversion of normalized paths to project
root-based paths occurs after all sources are processed. That conversion
is already implemented, so this PR doesn't need to address it. ([It
seems like the path conversion needs
improvement](foundry-rs/foundry#5307), but it
is a separate issue and should be handled in another PR.)


https://github.com/foundry-rs/compilers/blob/b1561d807c246c066c7c4b0c72c8eb64c696a43d/src/lib.rs#L514-L525

With the changes proposed in this PR, I confirmed the fix of the issue
by building `forge` with this patch and testing both the reproduction
case and my project. The resulting json matches the manually edited json
diff mentioned earlier.

A potential downside of this approach is that the same file could be
represented differently in the json if accessed via both symlinked and
real paths. However, I see no issues with this, and it aligns with the
behavior of the compiler's virtual filesystem.

src/config.rs

@@ -230,7 +230,7 @@ impl ProjectPathsConfig {
         if component == Component::CurDir || component == Component::ParentDir {
             // if the import is relative we assume it's already part of the processed input
             // file set
-            utils::canonicalize(cwd.join(import)).map_err(|err| {
+            utils::normalize_solidity_import_path(cwd, import).map_err(|err| {
                 SolcError::msg(format!("failed to resolve relative import \"{err:?}\""))
             })
         } else {
@@ -253,7 +253,7 @@ impl ProjectPathsConfig {
                 // also try to resolve absolute imports from the project paths
                 for path in [&self.root, &self.sources, &self.tests, &self.scripts] {
                     if cwd.starts_with(path) {
-                        if let Ok(import) = utils::canonicalize(path.join(import)) {
+                        if let Ok(import) = utils::normalize_solidity_import_path(path, import) {
                             return Ok(import);
                         }
                     }
@@ -1026,4 +1026,65 @@ mod tests {
             Path::new("/root/test/")
         );
     }
+
+    #[test]
+    fn can_resolve_import() {
+        let dir = tempfile::tempdir().unwrap();
+        let config = ProjectPathsConfig::builder().root(dir.path()).build().unwrap();
+        config.create_all().unwrap();
+
+        fs::write(config.sources.join("A.sol"), r"pragma solidity ^0.8.0; contract A {}").unwrap();
+
+        // relative import
+        assert_eq!(
+            config
+                .resolve_import_and_include_paths(
+                    &config.sources,
+                    Path::new("./A.sol"),
+                    &mut Default::default(),
+                )
+                .unwrap(),
+            config.sources.join("A.sol")
+        );
+
+        // direct import
+        assert_eq!(
+            config
+                .resolve_import_and_include_paths(
+                    &config.sources,
+                    Path::new("src/A.sol"),
+                    &mut Default::default(),
+                )
+                .unwrap(),
+            config.sources.join("A.sol")
+        );
+    }
+
+    #[test]
+    fn can_resolve_remapped_import() {
+        let dir = tempfile::tempdir().unwrap();
+        let mut config = ProjectPathsConfig::builder().root(dir.path()).build().unwrap();
+        config.create_all().unwrap();
+
+        let dependency = config.root.join("dependency");
+        fs::create_dir(&dependency).unwrap();
+        fs::write(dependency.join("A.sol"), r"pragma solidity ^0.8.0; contract A {}").unwrap();
+
+        config.remappings.push(Remapping {
+            context: None,
+            name: "@dependency/".into(),
+            path: "dependency/".into(),
+        });
+
+        assert_eq!(
+            config
+                .resolve_import_and_include_paths(
+                    &config.sources,
+                    Path::new("@dependency/A.sol"),
+                    &mut Default::default(),
+                )
+                .unwrap(),
+            dependency.join("A.sol")
+        );
+    }
 }

src/utils.rs

@@ -177,6 +177,81 @@ pub fn canonicalize(path: impl AsRef<Path>) -> Result<PathBuf, SolcIoError> {
     res.map_err(|err| SolcIoError::new(err, path))
 }
 
+/// Returns a normalized Solidity file path for the given import path based on the specified
+/// directory.
+///
+/// This function resolves `./` and `../`, but, unlike [`canonicalize`], it does not resolve
+/// symbolic links.
+///
+/// The function returns an error if the normalized path does not exist in the file system.
+///
+/// See also: <https://docs.soliditylang.org/en/v0.8.23/path-resolution.html>
+pub fn normalize_solidity_import_path(
+    directory: impl AsRef<Path>,
+    import_path: impl AsRef<Path>,
+) -> Result<PathBuf, SolcIoError> {
+    let original = directory.as_ref().join(import_path);
+    let cleaned = clean_solidity_path(&original);
+
+    // this is to align the behavior with `canonicalize`
+    use path_slash::PathExt;
+    let normalized = PathBuf::from(dunce::simplified(&cleaned).to_slash_lossy().as_ref());
+
+    // checks if the path exists without reading its content and obtains an io error if it doesn't.
+    normalized.metadata().map(|_| normalized).map_err(|err| SolcIoError::new(err, original))
+}
+
+// This function lexically cleans the given path.
+//
+// It performs the following transformations for the path:
+//
+// * Resolves references (current directories (`.`) and parent (`..`) directories).
+// * Reduces repeated separators to a single separator (e.g., from `//` to `/`).
+//
+// This transformation is lexical, not involving the file system, which means it does not account
+// for symlinks. This approach has a caveat. For example, consider a filesystem-accessible path
+// `a/b/../c.sol` passed to this function. It returns `a/c.sol`. However, if `b` is a symlink,
+// `a/c.sol` might not be accessible in the filesystem in some environments. Despite this, it's
+// unlikely that this will pose a problem for our intended use.
+//
+// # How it works
+//
+// The function splits the given path into components, where each component roughly corresponds to a
+// string between separators. It then iterates over these components (starting from the leftmost
+// part of the path) to reconstruct the path. The following steps are applied to each component:
+//
+// * If the component is a current directory, it's removed.
+// * If the component is a parent directory, the following rules are applied:
+//     * If the preceding component is a normal, then both the preceding normal component and the
+//       parent directory component are removed. (Examples of normal components include `a` and `b`
+//       in `a/b`.)
+//     * Otherwise (if there is no preceding component, or if the preceding component is a parent,
+//       root, or prefix), it remains untouched.
+// * Otherwise, the component remains untouched.
+//
+// Finally, the processed components are reassembled into a path.
+fn clean_solidity_path(original_path: impl AsRef<Path>) -> PathBuf {
+    let mut new_path = Vec::new();
+
+    for component in original_path.as_ref().components() {
+        match component {
+            Component::Prefix(..) | Component::RootDir | Component::Normal(..) => {
+                new_path.push(component);
+            }
+            Component::CurDir => {}
+            Component::ParentDir => {
+                if let Some(Component::Normal(..)) = new_path.last() {
+                    new_path.pop();
+                } else {
+                    new_path.push(component);
+                }
+            }
+        }
+    }
+
+    new_path.iter().collect()
+}
+
 /// Returns the same path config but with canonicalized paths.
 ///
 /// This will take care of potential symbolic linked directories.
@@ -228,7 +303,7 @@ pub fn resolve_library(libs: &[impl AsRef<Path>], source: impl AsRef<Path>) -> O
 /// until the `root` is reached.
 ///
 /// If an existing file under `root` is found, this returns the path up to the `import` path and the
-/// canonicalized `import` path itself:
+/// normalized `import` path itself:
 ///
 /// For example for following layout:
 ///
@@ -247,7 +322,7 @@ pub fn resolve_absolute_library(
 ) -> Option<(PathBuf, PathBuf)> {
     let mut parent = cwd.parent()?;
     while parent != root {
-        if let Ok(import) = canonicalize(parent.join(import)) {
+        if let Ok(import) = normalize_solidity_import_path(parent, import) {
             return Some((parent.to_path_buf(), import));
         }
         parent = parent.parent()?;
@@ -654,6 +729,99 @@ pragma solidity ^0.8.0;
         assert_eq!(Some("^0.8.0"), find_version_pragma(s).map(|s| s.as_str()));
     }
 
+    #[test]
+    fn can_normalize_solidity_import_path() {
+        let dir = tempfile::tempdir().unwrap();
+        let dir_path = dir.path();
+
+        // File structure:
+        //
+        // `dir_path`
+        // └── src (`cwd`)
+        //     ├── Token.sol
+        //     └── common
+        //         └── Burnable.sol
+
+        fs::create_dir_all(dir_path.join("src/common")).unwrap();
+        fs::write(dir_path.join("src/Token.sol"), "").unwrap();
+        fs::write(dir_path.join("src/common/Burnable.sol"), "").unwrap();
+
+        // assume that the import path is specified in Token.sol
+        let cwd = dir_path.join("src");
+
+        assert_eq!(
+            normalize_solidity_import_path(&cwd, "./common/Burnable.sol").unwrap(),
+            dir_path.join("src/common/Burnable.sol"),
+        );
+
+        assert!(normalize_solidity_import_path(&cwd, "./common/Pausable.sol").is_err());
+    }
+
+    // This test is exclusive to unix because creating a symlink is a privileged action on Windows.
+    // https://doc.rust-lang.org/std/os/windows/fs/fn.symlink_dir.html#limitations
+    #[test]
+    #[cfg(unix)]
+    fn can_normalize_solidity_import_path_symlink() {
+        let dir = tempfile::tempdir().unwrap();
+        let dir_path = dir.path();
+
+        // File structure:
+        //
+        // `dir_path`
+        // ├── dependency
+        // │   └── Math.sol
+        // └── project
+        //     ├── node_modules
+        //     │   └── dependency -> symlink to actual 'dependency' directory
+        //     └── src (`cwd`)
+        //         └── Token.sol
+
+        fs::create_dir_all(dir_path.join("project/src")).unwrap();
+        fs::write(dir_path.join("project/src/Token.sol"), "").unwrap();
+        fs::create_dir(dir_path.join("project/node_modules")).unwrap();
+
+        fs::create_dir(dir_path.join("dependency")).unwrap();
+        fs::write(dir_path.join("dependency/Math.sol"), "").unwrap();
+
+        std::os::unix::fs::symlink(
+            dir_path.join("dependency"),
+            dir_path.join("project/node_modules/dependency"),
+        )
+        .unwrap();
+
+        // assume that the import path is specified in Token.sol
+        let cwd = dir_path.join("project/src");
+
+        assert_eq!(
+            normalize_solidity_import_path(cwd, "../node_modules/dependency/Math.sol").unwrap(),
+            dir_path.join("project/node_modules/dependency/Math.sol"),
+        );
+    }
+
+    #[test]
+    fn can_clean_solidity_path() {
+        assert_eq!(clean_solidity_path("a"), PathBuf::from("a"));
+        assert_eq!(clean_solidity_path("./a"), PathBuf::from("a"));
+        assert_eq!(clean_solidity_path("../a"), PathBuf::from("../a"));
+        assert_eq!(clean_solidity_path("/a/"), PathBuf::from("/a"));
+        assert_eq!(clean_solidity_path("//a"), PathBuf::from("/a"));
+        assert_eq!(clean_solidity_path("a/b"), PathBuf::from("a/b"));
+        assert_eq!(clean_solidity_path("a//b"), PathBuf::from("a/b"));
+        assert_eq!(clean_solidity_path("/a/b"), PathBuf::from("/a/b"));
+        assert_eq!(clean_solidity_path("a/./b"), PathBuf::from("a/b"));
+        assert_eq!(clean_solidity_path("a/././b"), PathBuf::from("a/b"));
+        assert_eq!(clean_solidity_path("/a/../b"), PathBuf::from("/b"));
+        assert_eq!(clean_solidity_path("a/./../b/."), PathBuf::from("b"));
+        assert_eq!(clean_solidity_path("a/b/c"), PathBuf::from("a/b/c"));
+        assert_eq!(clean_solidity_path("a/b/../c"), PathBuf::from("a/c"));
+        assert_eq!(clean_solidity_path("a/b/../../c"), PathBuf::from("c"));
+        assert_eq!(clean_solidity_path("a/b/../../../c"), PathBuf::from("../c"));
+        assert_eq!(
+            clean_solidity_path("a/../b/../../c/./Token.sol"),
+            PathBuf::from("../c/Token.sol")
+        );
+    }
+
     #[test]
     fn can_find_ancestor() {
         let a = Path::new("/foo/bar/bar/test.txt");

tests/project.rs

@@ -1684,6 +1684,74 @@ fn can_compile_std_json_input() {
     }
 }
 
+// This test is exclusive to unix because creating a symlink is a privileged action on windows.
+// https://doc.rust-lang.org/std/os/windows/fs/fn.symlink_dir.html#limitations
+#[test]
+#[cfg(unix)]
+fn can_create_standard_json_input_with_symlink() {
+    let mut project = TempProject::dapptools().unwrap();
+    let dependency = TempProject::dapptools().unwrap();
+
+    // File structure:
+    //
+    // project
+    // ├── node_modules
+    // │   └── dependency -> symlink to actual 'dependency' directory
+    // └── src
+    //     └── A.sol
+    //
+    // dependency
+    // └── src
+    //     ├── B.sol
+    //     └── C.sol
+
+    fs::create_dir(project.root().join("node_modules")).unwrap();
+
+    std::os::unix::fs::symlink(dependency.root(), project.root().join("node_modules/dependency"))
+        .unwrap();
+    project.project_mut().paths.remappings.push(Remapping {
+        context: None,
+        name: "@dependency/".into(),
+        path: "node_modules/dependency/".into(),
+    });
+
+    project
+        .add_source(
+            "A",
+            r"pragma solidity >=0.8.0; import '@dependency/src/B.sol'; contract A is B {}",
+        )
+        .unwrap();
+    dependency
+        .add_source("B", r"pragma solidity >=0.8.0; import './C.sol'; contract B is C {}")
+        .unwrap();
+    dependency.add_source("C", r"pragma solidity >=0.8.0; contract C {}").unwrap();
+
+    // solc compiles using the host file system; therefore, this setup is considered valid
+    project.assert_no_errors();
+
+    // can create project root based paths
+    let std_json =
+        project.project().standard_json_input(project.sources_path().join("A.sol")).unwrap();
+    assert_eq!(
+        std_json.sources.iter().map(|(path, _)| path.clone()).collect::<Vec<_>>(),
+        vec![
+            PathBuf::from("src/A.sol"),
+            PathBuf::from("node_modules/dependency/src/B.sol"),
+            PathBuf::from("node_modules/dependency/src/C.sol")
+        ]
+    );
+
+    // can compile using the created json
+    let compiler_errors = Solc::default()
+        .compile(&std_json)
+        .unwrap()
+        .errors
+        .into_iter()
+        .filter_map(|e| if e.severity.is_error() { Some(e.message) } else { None })
+        .collect::<Vec<_>>();
+    assert!(compiler_errors.is_empty(), "{:?}", compiler_errors);
+}
+
 #[test]
 fn can_compile_model_checker_sample() {
     let root = PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("test-data/model-checker-sample");