Unlink a symlink with no target before writing the generated file

[thedavidmeister]

LibFs.buildFileForContract removes whatever is at the generated path before it
writes, so that a symlink at that path is replaced by a regular file rather than
written through to its target. It asked vm.exists, which answers for what a
path resolves to: a symlink whose target does not exist resolves to nothing and
reports absent, so the removal was skipped and vm.writeFile followed the link
and created the target instead of a file at the path.

LibFs.isPresent now answers that question about the path itself.
vm.readLink reverts unless the path is a symlink, so it sees exactly the case
vm.exists does not, and the two together answer true for a file, a directory,
a symlink that resolves and a symlink that does not. buildFileForContract asks
isPresent, and its NatSpec now states the symlink-with-no-target case that the
behaviour covers.

From the whole-repo audit at 7aa85a4. There is no issue attached to this one.

ffi = true in foundry.toml

forge-std 1.16.1 has no cheatcode that creates a symlink, so nothing could put
one in front of this code at all — on main, deleting the entire removal block
leaves the suite green. The tests here build symlinks with ln and read them
back with readlink, both of which need ffi = true.

The ledger for that line:

• Build cost. One config line. The narrower route, forge test --ffi, is
  not available to this repo: CI runs rainix's rainix-sol reusable, which runs
  plain forge test, and shared CI is rainix's to change.
• Carrying cost. Any test in this repo can now run a command. The workflows
  that run it are on: [push] with secrets: inherit, so the exposure is
  bounded by push access — which already carries the ability to edit those
  workflows. foundry.toml is listed in .soldeerignore, so no consumer of the
  published package inherits the setting. raindex and rain.math.float
  already set ffi = true in their default profile.
• Removal cost. If a symlink-creating cheatcode ever lands, three tryFfi
  helpers in one test file are what changes, and the config line goes with them.
  Nothing under src/ uses ffi.

Every path the tests hand to a command is built inside the test from a bare
name, so no test in the file can name a path outside src/generated. Setup and
teardown use rm -rf rather than the cheatcodes, because a cleanup that leaned
on the behaviour under test would leave the tree dirty exactly when the test
fails.

Placement, and what is left to issue 66

The new file is at test/src/lib/LibFs.isPresent.t.sol, the mirror path for
src/lib/LibFs.sol, rather than in test/lib/ next to the 16 files that
#56 moves there. It touches
no existing file under test/, so it neither conflicts with that PR nor with
the other open PRs in the same tree.

#66 owns the removal
block's coverage in general. This PR covers the
symlink-with-no-target case end to end, and isPresent over its whole domain.
Left there: the write's behaviour for a symlink that does resolve (covered
here only for isPresent's own answer, not through buildFileForContract), and
the block's other claims.

Issue 66's own fix proposes ffi = true as well, so whichever of the two lands
second gets a one-line conflict in foundry.toml and should keep the surviving
comment rather than the second copy of the setting.

QA

• Discriminating tests: testBuildFileForContractReplacesDanglingSymlink,
  testIsPresentDanglingSymlink, testIsPresentNothing, testIsPresentFile,
  testIsPresentDirectory, testIsPresentSymlink. The first was written and
  run against unmodified LibFs and failed there —
  VM::exists("src/generated/LibFsIsPresentDangling.sol") → false, then
  VM::writeFile to that path, then
  VM::exists("src/generated/LibFsIsPresentDanglingTarget.txt") → true, giving
  [FAIL: the write followed the link to its target] testBuildFileForContractReplacesDanglingSymlink() and Suite result: FAILED. 0 passed; 1 failed. On disk after that run the path was still a symlink and
  the 382-byte generated file was at the target. Its preconditions passed in
  that same run, so the link was really created and really dangling: ln exit
  code 0, readlink exit code 0, vm.exists false for both the link and its
  target. The other five could not run against base — the function they call
  did not exist — and each is instead pinned by the mutants below. After the
  fix: Ran 17 test suites: 140 tests passed, 0 failed, 0 skipped, against 134
  on main, with no existing test changed, and git status --porcelain after
  the run lists only the files this PR changes.
• Mutations applied, each to the committed fix, confirmed landed with git diff --numstat (1 line) before the run, full suite run, then git checkout -- to
  restore. Every run reports 140 total tests, so the suite ran in all four:
  • isPresent: if (vm.exists(path)) → if (false) — killed, 3 failed
    (testIsPresentFile, testIsPresentDirectory, testIsPresentSymlink).
  • isPresent: try branch returns false — killed, 2 failed
    (testIsPresentDanglingSymlink,
    testBuildFileForContractReplacesDanglingSymlink).
  • isPresent: catch branch returns true — killed, 10 failed
    (testIsPresentNothing, testBuildFileForContractReplacesDanglingSymlink,
    and 8 pre-existing tests, all reverting in vm.removeFile on a path holding
    nothing).
  • buildFileForContract: isPresent(vm, path) → vm.exists(path), i.e. the
    behaviour before this PR — killed, 1 failed
    (testBuildFileForContractReplacesDanglingSymlink).
• Oracle: the filesystem, read by ln, readlink and rm rather than by the
  cheatcodes the library calls. vm.readLink is what isPresent uses, so the
  tests never assert with it; whether a path is a symlink is asked of the
  readlink binary, and whether the write followed the link is asked as whether
  the target file exists at all. The generated content is compared against a
  second contract written to a path that held nothing, so the claim is that the
  two cases produce the same file rather than that particular bytes appear.
• Category check: the finding is about vm.exists answering for a path's
  target instead of the path. The category is every kind of thing that can
  occupy a path, so isPresent is asserted over all of them — nothing, a file,
  a directory, a symlink that resolves, a symlink that does not — not only the
  dangling case that motivated it, and the write is asserted end to end for that
  case. Left to LibFs.buildFileForContract's unlink block has no test, and no test in this repo can currently reach it #66 as
  described above.
• Static: forge fmt --check exits 0. forge lint over the two changed files
  reports nothing. reuse lint is compliant, 48/48 files. slither . reports
  0 findings: it read the ignored vm.readLink return as unused-return,
  which is what the call is for, so that line carries the org's
  //slither-disable-next-line unused-return and a comment saying why the
  target is not read.

[coderabbitai]

Warning

Review limit reached

@thedavidmeister, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 55 minutes

Limit details: You’ve used all 1 included review currently available under your plan.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: c777f729-9550-4f90-822c-30d8aa9ba31b

📥 Commits

Reviewing files that changed from the base of the PR and between 935c725 and ed3bca5.

📒 Files selected for processing (3)

• foundry.toml
• src/lib/LibFs.sol
• test/src/lib/LibFs.isPresent.t.sol

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}