`freeze()`'s write path and its `SnapshotAlreadyFrozen` append-only guard are untested

[thedavidmeister]

Two coverage findings with one blocker and one fix: freeze can only be pointed at the real src/generated/ record, so neither its write path nor its append-only guard can be driven from a test.

---

Finding cov-03

Audit finding cov-03 — dimension 2, severity MEDIUM. Whole-repo audit pass 1 at 440e90b5.

src/lib/LibRainDeploySnapshot.sol:745-747

Problem

grep -rn SnapshotAlreadyFrozen test/ returns nothing. The error is declared, documented as the thing that makes a release "cut once", and no test ever reaches it. It is the only protection on the immutability of src/generated/<tag>/, which is the record consumers pin their bytecode against: if it stopped firing, re-running cutRelease() would silently overwrite a published release's frozen snapshot with whatever the candidate currently holds.

Every other guard in this library has a test whose stated reason is that "a guard nobody has seen fire is a guard nobody knows works" — this one is the exception, and it is the guard with the most durable consequence. It is untestable today for the same reason as cov-02: freeze can only be pointed at src/generated/.

Proposed fix

With the root seam from cov-02 in place, add to test/src/lib/LibRainDeploySnapshot.t.sol:

    /// Where the re-cut fixture's record is built. Its own tree: this one is
    /// deliberately left frozen between the two calls, so no other test may
    /// share it.
    string constant RECUT_FIXTURE_ROOT = "test/generated-recut";

    /// External wrapper so `vm.expectRevert` lands at the right call depth.
    /// @param root The record root to freeze into.
    /// @param contractNames The contracts to freeze.
    function externalFreezeAt(string memory root, string[] memory contractNames) external {
        LibRainDeploySnapshot.freeze(vm, root, noRegeneration, contractNames);
    }

    /// A release is cut ONCE. Re-cutting a tag that already has a record MUST
    /// be refused, naming the tag and the directory, and MUST leave the
    /// original record exactly as it was — it is what consumers of that
    /// release pin against, and a second cut would replace it with whatever
    /// the candidate currently is.
    function testFreezeRefusesARecutRelease() external {
        string memory tag = LibRainDeploySnapshot.deployTag(vm);
        string memory frozenDir = string.concat(RECUT_FIXTURE_ROOT, "/", tag);
        string memory frozenPath = string.concat(frozenDir, "/", FREEZE_CONTRACT, ".sol");

        writeFixture(
            string.concat(RECUT_FIXTURE_ROOT, "/", LibRainDeploySnapshot.CANDIDATE, "/", FREEZE_CONTRACT, ".sol")
        );
        string[] memory contractNames = new string[](1);
        contractNames[0] = FREEZE_CONTRACT;

        LibRainDeploySnapshot.freeze(vm, RECUT_FIXTURE_ROOT, noRegeneration, contractNames);
        string memory firstCut = vm.readFile(frozenPath);

        // The candidate moves on, exactly as source does between releases.
        //forge-lint: disable-next-line(unsafe-cheatcode)
        vm.writeFile(
            string.concat(RECUT_FIXTURE_ROOT, "/", LibRainDeploySnapshot.CANDIDATE, "/", FREEZE_CONTRACT, ".sol"),
            freshRolling()
        );

        vm.expectRevert(abi.encodeWithSelector(SnapshotAlreadyFrozen.selector, tag, frozenDir));
        this.externalFreezeAt(RECUT_FIXTURE_ROOT, contractNames);

        // Read while the fixture is still there, asserted once it is gone.
        string memory afterRefusal = vm.readFile(frozenPath);
        string[] memory record = LibRainDeploySnapshot.frozenSnapshotPaths(vm, RECUT_FIXTURE_ROOT);

        //forge-lint: disable-next-line(unsafe-cheatcode)
        vm.removeDir(RECUT_FIXTURE_ROOT, true);

        assertEq(afterRefusal, firstCut);
        assertNotEq(keccak256(bytes(afterRefusal)), keccak256(bytes(freshRolling())));
        assertEq(record.length, 1);
    }

    /// The refusal is about the DIRECTORY existing, not about what is in it, so
    /// an empty `<tag>/` left behind by anything refuses the real cut too —
    /// which is why `EmptyRelease` and the write ordering exist.
    function testFreezeRefusesATagDirectoryThatIsEmpty() external {
        string memory tag = LibRainDeploySnapshot.deployTag(vm);
        string memory frozenDir = string.concat(RECUT_FIXTURE_ROOT, "-empty/", tag);
        //forge-lint: disable-next-line(unsafe-cheatcode)
        vm.createDir(frozenDir, true);
        writeFixture(
            string.concat(
                RECUT_FIXTURE_ROOT, "-empty/", LibRainDeploySnapshot.CANDIDATE, "/", FREEZE_CONTRACT, ".sol"
            )
        );

        string[] memory contractNames = new string[](1);
        contractNames[0] = FREEZE_CONTRACT;

        vm.expectRevert(
            abi.encodeWithSelector(SnapshotAlreadyFrozen.selector, tag, frozenDir)
        );
        this.externalFreezeAt(string.concat(RECUT_FIXTURE_ROOT, "-empty"), contractNames);

        //forge-lint: disable-next-line(unsafe-cheatcode)
        vm.removeDir(string.concat(RECUT_FIXTURE_ROOT, "-empty"), true);
    }

SnapshotAlreadyFrozen must be added to the existing import block at the top of the file.

---

Verification — this finding survived an adversarial refutation pass

Could not refute. Verified against source:

• /home/gildlab/code/rain-deploy-audit/src/lib/LibRainDeploySnapshot.sol:742-750 — freeze opens with if (vm.exists(frozenDir)) revert SnapshotAlreadyFrozen(tag, frozenDir); at exactly the cited lines 745-747. The error is declared at line 29 with the documented "a release is cut once" rationale, and the freeze NatSpec (line 721) names it as the guard that makes the record append-only.
• grep -rn SnapshotAlreadyFrozen over the whole repo hits only src/lib/LibRainDeploySnapshot.sol (declaration, two doc mentions, the revert). Nothing in test/, nothing in script/.
• The only two freeze call sites in tests are /home/gildlab/code/rain-deploy-audit/test/src/lib/LibRainDeploySnapshot.t.sol:648 (externalFreeze(new string[](0)) → EmptyRelease) and :671 (NoSuchContract → NothingToFreeze). Both run with src/generated/0_1_5/ absent (ls src/generated = candidate only; foundry.toml version = 0.1.5, nothing released yet), so the exists-check is false in both. Deleting the guard entirely leaves both tests green — the mutant survives, so the gap is real and not covered indirectly.
• The test contract's own header states the standard the finding invokes ("a guard nobody has seen fire is a guard nobody knows works"), and the other two freeze guards each have a dedicated test, so this is the stated-standard exception rather than an imported expectation.
• freeze genuinely has no root parameter (it derives paths via dirForSnapshot/pathForSnapshot, hardcoded to LIB_FS_ROOT = "src/generated"), unlike frozenSnapshotPaths/writeReleasedSuitesLib/recordPathsForContract, which all take a recordRoot precisely so they can be tested off the real tree — so the finding's account of why it is hard to test is accurate. (Strictly it is testable without the seam by creating src/generated/0_1_5 and removing it, but that races the other test contracts forge runs in parallel against the real record — the documented reason FIXTURE_ROOT exists at :41-45 — so the seam is the right fix.)

Severity corrected MEDIUM → LOW. Value at risk is bounded: this is dev-time release tooling, not deployed contract code; no release has been cut yet, so no frozen record currently exists to overwrite; cutRelease() is a manual, once-per-release human action against a committed tree, so an overwrite shows up as a modified src/generated/<tag>/ file in the release diff; and a record overwritten with a different candidate derives a different address, which RainDeployVerifyChain (group 4, released-only) red-lines because that address is not live on any supported network. The guard is worth a test, but its failure is neither silent nor unrecoverable in production.

---

Finding cov-02

Audit finding cov-02 — dimension 2, severity LOW. Whole-repo audit pass 1 at 440e90b5.

src/lib/LibRainDeploySnapshot.sol:742-771

Problem

LibRainDeploySnapshotTest drives only the two guards that fire BEFORE anything is written (EmptyRelease, NothingToFreeze), and both do so with noRegeneration(). Nothing anywhere calls freeze through to a successful cut, so none of this is exercised: the rolling snapshots are read back and copied into <tag>/, the copies are the bytes the regeneration just wrote, and the resulting directory is a release the record walk finds.

The ordering is the property that matters most and is the one entirely unasserted. The docstring states that freeze takes the regeneration as an argument and runs it FIRST precisely because freezing a stale candidate is silent — and with noRegeneration() as the only regeneration ever passed, a freeze that read the rolling files before calling regenerate() would pass every existing test. A stale freeze publishes a release whose recorded bytes are not the bytes that release deploys.

The write path is structurally undrivable today: dirForSnapshot hardcodes src/generated/, so a success-path test would create a real <tag>/ directory that RainDeployVerifySnapshot.testEveryFrozenSnapshotIsReleased walks from contracts forge runs in parallel. That is why the gap exists, and the fix has to open the same seam frozenSnapshotPaths and writeReleasedSuitesLib already have.

Proposed fix

1. Give freeze the record root the rest of the library already takes as a parameter, keeping the real caller unchanged:

    // src/lib/LibRainDeploySnapshot.sol
    function freeze(Vm vm, function() internal regenerate, string[] memory contractNames) internal {
        freeze(vm, LIB_FS_ROOT, regenerate, contractNames);
    }

    /// @param root The record root — `LIB_FS_ROOT` for a repo's real record. A
    /// parameter for the same reason `frozenSnapshotPaths` takes one: a writer
    /// that can only be pointed at the real record can only be tested against
    /// it, and the real record is one a test must not leave a release in.
    function freeze(Vm vm, string memory root, function() internal regenerate, string[] memory contractNames)
        internal
    {
        string memory tag = deployTag(vm);
        string memory frozenDir = string.concat(root, "/", tag);
        if (vm.exists(frozenDir)) {
            revert SnapshotAlreadyFrozen(tag, frozenDir);
        }
        if (contractNames.length == 0) {
            revert EmptyRelease(tag);
        }

        regenerate();

        string[] memory records = new string[](contractNames.length);
        for (uint256 i = 0; i < contractNames.length; i++) {
            string memory rollingPath = string.concat(root, "/", CANDIDATE, "/", contractNames[i], ".sol");
            if (!vm.exists(rollingPath)) {
                revert NothingToFreeze(rollingPath);
            }
            records[i] = vm.readFile(rollingPath);
        }

        //forge-lint: disable-next-line(unsafe-cheatcode)
        vm.createDir(frozenDir, true);
        for (uint256 i = 0; i < contractNames.length; i++) {
            //forge-lint: disable-next-line(unsafe-cheatcode)
            vm.writeFile(string.concat(frozenDir, "/", contractNames[i], ".sol"), records[i]);
        }
    }

2. Add to test/src/lib/LibRainDeploySnapshot.t.sol:

    /// Where the freeze fixture's record is built. Its own tree, for the same
    /// reason `RELEASED_FIXTURE_ROOT` is not `FIXTURE_ROOT`, and NOT
    /// `src/generated`: a transient `<tag>/` there is a release the inherited
    /// record check has to fail on, from contracts forge runs in parallel.
    string constant FREEZE_FIXTURE_ROOT = "test/generated-freeze";

    /// The contract the freeze fixture cuts.
    string constant FREEZE_CONTRACT = "Marked";

    /// What the regeneration writes. Split so `reuse lint` reads this as a
    /// fixture rather than as this file's own license declaration.
    function freshRolling() internal pure returns (string memory) {
        return string.concat(
            "// SPDX-License", "-Identifier: LicenseRef-DCL-1.0\n",
            "address constant DEPLOYED_ADDRESS = address(0xfresh0000000000000000000000000000000fee);\n"
        );
    }

    /// What is on disk before the call, and what a freeze that read before
    /// regenerating would copy.
    function staleRolling() internal pure returns (string memory) {
        return string.concat(
            "// SPDX-License", "-Identifier: LicenseRef-DCL-1.0\n",
            "address constant DEPLOYED_ADDRESS = address(0x5741e000000000000000000000000000000000ee);\n"
        );
    }

    /// A regeneration that really regenerates, so "the guard fired FIRST" is no
    /// longer the only thing a freeze test can observe.
    function regenerateFreezeFixture() internal {
        //forge-lint: disable-next-line(unsafe-cheatcode)
        vm.writeFile(
            string.concat(FREEZE_FIXTURE_ROOT, "/", LibRainDeploySnapshot.CANDIDATE, "/", FREEZE_CONTRACT, ".sol"),
            freshRolling()
        );
    }

    /// A freeze MUST copy the bytes the regeneration wrote, not the bytes that
    /// were on disk when it was called. Freezing a stale candidate is silent —
    /// the immutability check only fires on a re-cut, which is too late — so
    /// the ordering is the whole of what makes a release describe itself.
    function testFreezeCopiesTheRegeneratedRollingSnapshot() external {
        string memory tag = LibRainDeploySnapshot.deployTag(vm);
        writeFixture(
            string.concat(FREEZE_FIXTURE_ROOT, "/", LibRainDeploySnapshot.CANDIDATE, "/", FREEZE_CONTRACT, ".sol")
        );
        //forge-lint: disable-next-line(unsafe-cheatcode)
        vm.writeFile(
            string.concat(FREEZE_FIXTURE_ROOT, "/", LibRainDeploySnapshot.CANDIDATE, "/", FREEZE_CONTRACT, ".sol"),
            staleRolling()
        );

        string[] memory contractNames = new string[](1);
        contractNames[0] = FREEZE_CONTRACT;
        LibRainDeploySnapshot.freeze(vm, FREEZE_FIXTURE_ROOT, regenerateFreezeFixture, contractNames);

        string memory frozenPath = string.concat(FREEZE_FIXTURE_ROOT, "/", tag, "/", FREEZE_CONTRACT, ".sol");
        bool frozenExists = vm.exists(frozenPath);
        string memory frozen = frozenExists ? vm.readFile(frozenPath) : "";
        // The cut is a release the record walk finds, under the tag the version
        // maps to and nowhere else.
        string[] memory record = LibRainDeploySnapshot.frozenSnapshotPaths(vm, FREEZE_FIXTURE_ROOT);
        // And the rolling snapshot is the regenerated one, still in place.
        string memory rolling = vm.readFile(
            string.concat(FREEZE_FIXTURE_ROOT, "/", LibRainDeploySnapshot.CANDIDATE, "/", FREEZE_CONTRACT, ".sol")
        );

        //forge-lint: disable-next-line(unsafe-cheatcode)
        vm.removeDir(FREEZE_FIXTURE_ROOT, true);

        assertTrue(frozenExists);
        assertEq(frozen, freshRolling());
        assertNotEq(keccak256(bytes(frozen)), keccak256(bytes(staleRolling())));
        assertEq(rolling, freshRolling());
        assertEq(record.length, 1);
        assertEq(record[0], frozenPath);
    }

    /// A release naming SEVERAL contracts MUST freeze every one of them. A
    /// contract regenerated but absent from the record is a contract silently
    /// missing from the release, and a tag that never held it has nothing
    /// missing from it for anything downstream to notice.
    function testFreezeCutsEveryNamedContract() external {
        string memory tag = LibRainDeploySnapshot.deployTag(vm);
        string[] memory contractNames = new string[](2);
        contractNames[0] = FREEZE_CONTRACT;
        contractNames[1] = "MarkedSecond";
        for (uint256 i = 0; i < contractNames.length; i++) {
            writeFixture(
                string.concat(
                    FREEZE_FIXTURE_ROOT, "/", LibRainDeploySnapshot.CANDIDATE, "/", contractNames[i], ".sol"
                )
            );
        }

        LibRainDeploySnapshot.freeze(vm, FREEZE_FIXTURE_ROOT, noRegeneration, contractNames);

        string[] memory record = LibRainDeploySnapshot.frozenSnapshotPaths(vm, FREEZE_FIXTURE_ROOT);
        bool first = holdsPath(record, string.concat(FREEZE_FIXTURE_ROOT, "/", tag, "/", contractNames[0], ".sol"));
        bool second = holdsPath(record, string.concat(FREEZE_FIXTURE_ROOT, "/", tag, "/", contractNames[1], ".sol"));

        //forge-lint: disable-next-line(unsafe-cheatcode)
        vm.removeDir(FREEZE_FIXTURE_ROOT, true);

        assertTrue(first);
        assertTrue(second);
        assertEq(record.length, 2);
    }

---

Verification — this finding survived an adversarial refutation pass

Verified against source, not refuted.

The factual claims hold. freeze at src/lib/LibRainDeploySnapshot.sol:742-771 is called from exactly two places: script/Build.sol:120 (cutRelease(), a tag/dispatch-only path nothing in forge test invokes) and test/src/lib/LibRainDeploySnapshot.t.sol:39 (externalFreeze, which hardcodes noRegeneration). Grepping the tree for freeze yields only two tests — testFreezeRefusesAnEmptyRelease (:644) and testFreezeLeavesNothingBehindWhenThereIsNothingToFreeze (:661) — and both revert before vm.createDir(frozenDir, true). SnapshotAlreadyFrozen is not even imported by the test file. So the createDir + copy loop (lines 765-770) is executed by no test, and src/generated/ holds only candidate/, so the write path has never run in this repo at all.

The ordering claim is the sharper half and also holds: moving regenerate(); (line 752) below the read loop is a mutation that survives the whole suite, since the only regeneration any test passes is a no-op. The test file concedes this at :30-33 ("a no-op is what makes 'the guard fired' and 'the guard fired FIRST' the same observation"), while the library header and script/Build.sol:106-108 assert the ordering as load-bearing. An asserted property with a surviving mutant is a real dimension-2 gap, and nothing in CLAUDE.md documents this path as deliberately unexercised.

Severity corrected MEDIUM -> LOW, on value-at-risk:

1. The silent failure mode it leans on (stale freeze) is independently prevented on every push. RegistryDeploySnapshotTest inherits testSnapshotMatchesSource (RainDeployVerifySnapshot.sol:253) and testSnapshotInternallyConsistent (:224); the candidate suite's creationCode/storedRuntimeCode are imported from the COMMITTED src/generated/candidate/*.sol while sourceCreationCode is type(X).creationCode (RegistryDeploySuites.sol:110-120). In any CI-green tree the on-disk candidate is byte-equivalent to what regenerate() would write, so read-before-regenerate is unobservable. A stale freeze is reachable only by tagging a commit whose CI is already red.
2. A write-path defect fails loudly where it actually runs: cutRelease() regenerates the released-suites libs FROM the record immediately after (Build.sol:120-121), then testEveryFrozenSnapshotIsReleased and RainDeployVerifyChain check that record against the declaration and the live chains inside the same rainix-tag-release run (.github/workflows/package-release.yaml). The cost of the gap is a red release job, not a silently bad published record.
3. Nothing here is deployed bytecode or on-chain authority; it is build-time file writing.

One defect in the proposed fix the parent should not land as written: it parameterizes where freeze WRITES, citing frozenSnapshotPaths and writeReleasedSuitesLib as precedent — but both parameterize a READ root (writeReleasedSuitesLib still writes to a hardcoded src/lib/LibReleased.sol). writeSnapshot's docstring (LibRainDeploySnapshot.sol:252-259) states the opposite rule for writers: "There is no output root to choose... writing this repo's record somewhere else is not [a thing]". And the fix replaces pathForSnapshot(CANDIDATE, ...) with a hand string.concat(root, "/", CANDIDATE, ...), exactly what pathForSnapshot's docstring (:178-181) forbids: "Two spellings of one path is how a freeze silently reads nothing." It is equivalent today only because LibFs.pathForContract is src/generated/<name>.sol, so any future drift would be silent. The seam should be opened via root-aware dirForSnapshot/pathForSnapshot variants that preserve the single spelling, not by concatenating inside freeze.