artifactPath is a hand-written string nothing resolves, and half of it is load bearing for the shape test

[thedavidmeister]

Audit finding H6-06 — dimension 6, severity INFO. Whole-repo audit pass 1 at 440e90b5.

src/abstract/RegistryDeploySuites.sol:110-151

Problem

HAZARD: artifactPath is the one field of a suite that is neither derived nor checked against anything, yet its :<Name> half silently decides which snapshot the shape assertions attribute to which candidate — so a wrong path is both an unusable verification command and a name that is right only by accident.

SHAPE: DeploySuite.artifactPath is declared by hand ("src/concrete/AddressRegistry.sol:AddressRegistry" at line 118, "src/concrete/MigrationRegistry.sol:MigrationRegistry" at line 146) and RainDeploySuitesBase.sol:73-78 explains it is declared rather than derived because no naming convention recovers a grouped repo's paths — a good reason not to derive it, and not a reason to leave it unchecked. It is consumed in two places: LibRainDeploy.deployToNetworks prints it into the forge verify-contract command (src/lib/LibRainDeploy.sol:448-453), and GeneratedSnapshotShapeTest.candidateContractName splits it on : and uses the trailing token as the contract name that must match a file in src/generated/candidate/ (test/src/lib/GeneratedSnapshotShape.t.sol:126-129). Nothing asserts the path half names a file that exists, or that the file declares that contract.

SCENARIO: AddressRegistry.sol is moved into a subdirectory as the repo grows a second family of concretes, and every reference the compiler can see is updated — imports, remappings, type(X).creationCode — while this string, which no compiler reads, is left at src/concrete/AddressRegistry.sol:AddressRegistry. All green: the contract name half still matches the snapshot, so the shape test is satisfied, and no other test touches the field. It next matters during a deploy dispatch, where the printed manual verification command names a file that does not exist, at the exact moment somebody is trying to verify a freshly broadcast contract on five explorers.

STRUCTURAL FIX: assert the declared path resolves to a file that declares the named contract, for every candidate, so the field is checked rather than trusted.

Proposed fix

Add to test/src/lib/GeneratedSnapshotShape.t.sol (which already inherits RegistryDeploySuites and already derives the contract name from this field). fs_permissions already grants read-write on ./src:

/// PROPERTY: every declared artifact path resolves — `<path>:<Name>` where the
/// path is a file that exists and that file declares `<Name>`.
///
/// It is the one suite field nothing derives and nothing else checks, and it is
/// load bearing twice over: `LibRainDeploy` prints it as the explorer
/// verification command a human runs against a freshly broadcast contract, and
/// `candidateContractName` takes the contract this whole shape spec is about
/// out of it. A path left behind by a moved source file passes every other
/// assertion here, because the half that moved is the half nothing reads.
function testEveryCandidateArtifactPathResolves() external view {
    DeployCandidate[] memory candidates = checkedCandidateSuites();
    for (uint256 i = 0; i < candidates.length; i++) {
        string memory declared = candidates[i].snapshot.artifactPath;
        string[] memory parts = vm.split(declared, ":");
        assertEq(parts.length, 2, string.concat("artifact path is not <path>:<Name>: ", declared));
        assertTrue(vm.exists(parts[0]), string.concat("artifact path names no such file: ", declared));
        assertTrue(
            vm.contains(vm.readFile(parts[0]), string.concat("contract ", parts[1], " ")),
            string.concat("artifact path's file declares no such contract: ", declared)
        );
    }
}

---

Verification — this finding survived an adversarial refutation pass

SURVIVES, downgraded LOW -> INFO.

Verified against source. The mechanism is exactly as described: artifactPath is a hand-written literal (RegistryDeploySuites.sol:118, :146), declared rather than derived per the documented reason at RainDeploySuitesBase.sol:73-78, and has exactly two consumers - RainDeployBroadcast.run() passes suite.artifactPath into deployAndBroadcast -> deployToNetworks, where LibRainDeploy.sol:447-453 prints it into a console2.log of the forge verify-contract command; and GeneratedSnapshotShape.t.sol:126-129 splits it on ":" and takes the trailing token as the contract name.

Grepped the full test tree for any resolution of the path half (vm.exists / vm.isFile / readFile over a declared artifact path): nothing. The only assertions on the field are equality against a hardcoded literal (RainDeployBroadcast.t.sol:136) and pass-through equality (RainDeploySuitesBase.t.sol:60) - neither resolves the path. So the path half is genuinely unchecked, and the drift scenario is clean: foundry.toml sets bytecode_hash = "none" and cbor_metadata = false, so moving a source file changes no bytecode, no address, no codehash, and every group 1-4 check plus the whole shape spec stays green.

One overreach in the finding, not fatal: "a name that is right only by accident" is false. testEveryCandidateHasASnapshot asserts set equality between the names derived from artifactPath and the names walked out of src/generated/candidate/, in BOTH directions plus a count assertion, so the :Name half is cross-checked against the generator (and independently against Build.sol's GeneratedContract.contractName, which places that file). The finding's own scenario concedes this. The surviving claim is narrower than the title: only the path half is untrusted.

Severity corrected to INFO on value-at-risk, not on depth. Three things I confirmed bound the exposure to near zero: (1) the field is inert to the deployment - it never affects creation code, the derived address, the codehash, dependency checks, or any verification assertion, so nothing on chain and nothing in the record can be wrong because of it; (2) foundry.toml's [etherscan] block records that rainix-manual-sol-artifacts passes --verify by default, so this printed string is a human fallback for an automated verification step that has already run, not the primary path; (3) the failure is loud and non-silent - forge verify-contract --chain X <addr> <bad-path>:<Name> errors with no matching artifact rather than verifying something else, and a mismatched path:Name cannot resolve to a different contract because two same-named contracts cannot both be the artifact. Total cost of the hazard firing is a human retyping a path during a deploy they are already supervising. LOW implies some production value at risk; there is none.