Cover buildFileForContract's unlink with a hard link, and enable ffi to reach it

[thedavidmeister]

Closes #66

What changed

One test, testBuildFileForContractUnlinksBeforeWriting, in
test/lib/LibFs.buildFileForContract.t.sol, plus ffi = true in
foundry.toml so it can create the link it needs. src/lib/LibFs.sol is
untouched.

The test puts a hard link at the generated path: two names for one file. It
writes a sentinel through the other name, asserts the path reads it back (so
the two names are proven to be one file, not two with equal bytes), then runs
buildFileForContract. Afterwards the other name still holds the sentinel and
the path holds the generated file. Without the unlink, vm.writeFile truncates
and rewrites the file that is already at the path — which is the same file the
other name refers to — so the sentinel is replaced by the generated bytes and
the test fails.

The issue's proposed fix asserts behaviour this library does not have

#66 proposed a dangling-symlink test. Measured on 935c725, with ffi
enabled, that test does not pass and could not cover the block if it did:

• vm.exists resolves symlinks, so a dangling link at the path reports
  absent and the unlink block never runs. vm.writeFile then follows the link
  and creates its target. The issue's assertFalse(vm.exists(target)) is false
  on this tree — it asserts a fix, not current behaviour, and the block it is
  meant to cover is not reached on that input at all.
• A live symlink does not discriminate either. vm.removeFile resolves the
  link and removes its target, leaving the link in place and dangling; the
  following vm.writeFile then recreates that same target. With and without
  the unlink block, the generated bytes end up at the link's target and the
  path is still a symlink. Measured directly: after vm.removeFile(<symlink>),
  readDir still reports the path with isSymlink == true while the target is
  gone.

A hard link avoids both: no resolution happens, vm.removeFile unlinks the
path itself, and the second name is a witness for whether the write landed on a
new file or through the old one.

Decision: ffi = true in foundry.toml

forge-std 1.16.1 can read a link (readLink) but creates neither kind, and
there is no cheatcode that makes a file un-writable, so no in-process route
distinguishes "unlink then write" from "write". The block is either covered by
shelling out or not covered.

Build cost. One config line and three lines of string[] in the test.

Carrying cost. The grant is repo-wide: every test and script here can run
arbitrary commands as whoever runs forge test, including a reviewer running a
contributor's branch locally. Not mitigated by config — foundry has no per-test
ffi grant. Bounded by what is actually in the blast radius: forge test runs
only this repo's test/, not the one dependency's (forge-std 1.16.1), and
foundry.toml does not propagate to consumers of the published soldeer
package, so no downstream repo inherits this. Exactly one call site today, and
C1 in the matrix below shows the suite names it when the grant is removed.

Removal cost. Delete the line and the one test; the unlink block goes back
to uncovered. There is nothing else to unpick.

Routes rejected.

• forge test --ffi — rainix-sol-test.yaml runs plain forge test -vvv, so
  this needs a change in rainix's reusable workflow, which every consumer repo
  pins @main. That converts a one-repo grant into an org-wide grant on every
  Solidity suite. Strictly wider than the config line, and shared CI is not
  this repo's to change.
• [profile.ffi] + FOUNDRY_PROFILE=ffi — CI runs the default profile, so the
  test would revert in CI. Coverage that does not run in CI is not coverage.
• A committed symlink fixture — the run replaces it and no cheatcode can
  recreate it, so it works once and leaves the tree dirty.
• A directory at the path — reverts both with and without the unlink, so it
  discriminates only on a cheatcode's error string, and never exercises the
  write.

Found, not fixed, not filed

Both are in buildFileForContract's body and docstring, which #61 is
rewriting, so they are left to that change rather than resolved here. Measured
on 935c725:

1. LibFs.sol:49-51 states a guarantee the library does not provide. "a
   symlink there is replaced by a regular file rather than written through to
   its target" is false in every case, because vm.removeFile resolves the
   link and removes the target. End to end over a live symlink: the path is
   still a symlink afterwards and the generated file is at its target.
2. A dangling symlink at the path sends the write outside the path. The
   unlink is skipped (vm.exists resolves the link), and vm.writeFile
   creates the link's target — which the link can point anywhere, including
   outside src/generated, since fs_permissions is checked against the path
   string.

#64 owns vm.createDir coverage and is untouched here.

QA

• Discriminating tests: testBuildFileForContractUnlinksBeforeWriting - fails
  on base behaviour (verified by deleting the unlink block on this branch and
  running the full suite: 1 failed / 135, output quoted below; with the test
  excluded the same mutant leaves 134/134 green, which is the finding).
• Mutations applied: LibFs.sol:68-71 whole if (vm.exists(path)) { vm.removeFile(path); } deleted -> killed by
  testBuildFileForContractUnlinksBeforeWriting; LibFs.sol:70
vm.removeFile(path); deleted -> killed by the same test; LibFs.sol:68
  guard dropped (unconditional removeFile) -> killed by
  testBuildFileForContractFreshPath + 7 siblings; LibFs.sol:68 guard
  negated to !vm.exists(path) -> killed by those 8 plus the new test;
  foundry.toml ffi = true deleted -> killed by the new test
  (vm.ffi: FFI is disabled). Full table below.
• Oracle: the sentinel "SHARED" written through the other name of the hard
  link, and expectedFile(instance, body), which the test file rebuilds from
  literal text plus address.codehash rather than by calling LibCodeGen.
  Neither value comes from the code under test.
• Category check: LibFs.buildFileForContract's unlink block has no test, and no test in this repo can currently reach it #66 asks for coverage of the unlink block at
  LibFs.sol:68-71 and names ffi as a decision to make. Covered: the block
  is now killed by a test (M1, M2 below), and the ffi decision is taken with
  its ledger above. Not covered, deliberately: the issue's dangling-symlink
  test asserts behaviour this library does not have and never reaches the
  block — see the two measured defects under "Found, not fixed, not filed",
  which belong to LibFs.buildFileForContract unlinks the existing file before the content is computed, so a failed build destroys the previously generated file #61's rewrite of these lines. vm.createDir coverage is vm.createDir(GENERATED_DIR, true) in LibFs.buildFileForContract has no test: deleting it leaves 134/134 passing #64.

Evidence

• Full suite before, on a clean 935c725 clone:
  Ran 16 test suites: 134 tests passed, 0 failed, 0 skipped (134 total).

• Finding reproduced. Unlink block deleted, new test excluded:
  Ran 16 test suites: 134 tests passed, 0 failed, 0 skipped (134 total) —
  the mutant that motivated LibFs.buildFileForContract's unlink block has no test, and no test in this repo can currently reach it #66 survives the suite as it stands.

• Failing before. Same mutant, new test included:
  Ran 16 test suites: 134 tests passed, 1 failed, 0 skipped (135 total)

  [FAIL: the write went through the file that was already at the path: // SPDX-License-Identifier: LicenseRef-DCL-1.0
  // SPDX-FileCopyrightText: Copyright (c) 2020 Rain Open Source Software Ltd
  pragma solidity ^0.8.25;
  
  // THIS FILE IS AUTOGENERATED BY THE BUILD SCRIPT. DO NOT EDIT BY HAND.
  
  /// @dev Hash of the known bytecode.
  bytes32 constant BYTECODE_HASH = bytes32(0x8016e5305d25be28ac4d72616cfd999756e36eded3ec88562375e2e1c9eca84d);
  
  // unlink
   != SHARED] testBuildFileForContractUnlinksBeforeWriting() (gas: 64495)
  

• Passing after. Source restored, nix develop -c forge test:
  Ran 16 test suites in 2.26s: 135 tests passed, 0 failed, 0 skipped (135 total).

• Mutation matrix (nix develop -c forge test, full suite each time; every
  row printed a Ran 16 test suites line, so every row actually ran):

  #	Mutant	Result
  M1	whole if (vm.exists(path)) { vm.removeFile(path); } deleted	killed — 1 failed / 135, the new test
  M2	only vm.removeFile(path); deleted	killed — 1 failed / 135, the new test
  M3	guard dropped, removeFile unconditional	killed — 8 failed / 135, pre-existing tests (vm.removeFile: … No such file or directory); already covered by testBuildFileForContractFreshPath
  M4	guard negated to if (!vm.exists(path))	killed — 9 failed / 135, 8 pre-existing plus the new test
  C1	ffi = true removed from foundry.toml	killed — 1 failed / 135: vm.ffi: FFI is disabled; the grant is load bearing and has exactly one user

• nix develop -c forge fmt --check — clean, exit 0.

• nix develop -c slither . — 9 contracts with 98 detectors, 0 result(s) found.

• CI on this branch, GitHub Actions rainix / test / test (ubuntu-latest, nix
  sol-shell, plain forge test -vvv from the reusable — no workflow change):
  [PASS] testBuildFileForContractUnlinksBeforeWriting() (gas: 72135) and
  Ran 16 test suites in 3.31s: 135 tests passed, 0 failed, 0 skipped (135 total tests). ffi and ln both work there as configured.

• CodeRabbit: rate limited, not reviewed. Its comment reads "Review limit
  reached … Next review available in: 54 minutes", and the PR has zero review
  threads. Its green check is the absence of a review, not a passed one.

[coderabbitai]

Warning

Review limit reached

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

Next review available in: 53 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: 3cd0a904-89f3-4e78-9175-cbe0b8f45d58

📥 Commits

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

📒 Files selected for processing (2)

• foundry.toml
• test/lib/LibFs.buildFileForContract.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.}

[thedavidmeister]

Closing unmerged. Both halves of this PR are already on main, landed by
#110.

ffi = true is already there. #110 added it to [profile.default] in
foundry.toml, above fs_permissions. This PR's hunk adds a second ffi
key to the same table, below fs_permissions. The two hunks touch different
lines, so git merges them without a conflict and the result is a TOML file with
a duplicate key in one table — a parse error, discovered at the first forge
invocation on main rather than here.

The unlink block is already covered. #110 added
test/src/lib/LibFs.isPresent.t.sol::testBuildFileForContractReplacesDanglingSymlink,
which puts a dangling symlink at the generated path, runs buildFileForContract,
and asserts the link's target was never created and the path is no longer a
symlink. Delete the if (isPresent(vm, path)) { vm.removeFile(path); } block and
vm.writeFile follows the link and creates the target, which reds that assertion.

The reason the issue's own proposed test could not work at the time this PR was
written — vm.exists resolves symlinks, so a dangling link reads as absent and
the block is never reached — is what #110 fixed in src/, by introducing
LibFs.isPresent and guarding on that instead of vm.exists. This PR routed
around that defect with a hard link rather than fixing it.

The hard-link test also pins a property LibFs's NatSpec never promises: that
the write lands on a new inode. What is documented is that a symlink at the
path is replaced rather than written through, which is what the test on main
asserts.

Closes #66 is superseded — #66 is closed against #110.