Security hardening: recordCompany auth, CEI, Ownable2Step+timelock, EAS membership, namehash, bounded loops

.gitmodules

@@ -1,3 +1,6 @@
 [submodule "src/contracts/lib/forge-std"]
 	path = src/contracts/lib/forge-std
 	url = https://github.com/foundry-rs/forge-std
+[submodule "src/contracts/lib/openzeppelin-contracts"]
+	path = src/contracts/lib/openzeppelin-contracts
+	url = https://github.com/OpenZeppelin/openzeppelin-contracts

SECURITY-HARDENING.md

@@ -0,0 +1,59 @@
+# StartupChain security hardening
+
+> Author: Lucia (Zenbit) · Branch: `hardening/contract-and-eas-security` · **Sepolia only — no mainnet
+> moves.** This PR applies the transferable hardening patterns from the AxoloDAO best-of-4 review to
+> StartupChain. The two products stay fully decoupled (no shared code); only the *patterns* transfer.
+
+Each fix ships with a test that proves the fix. `forge test` = **61 passing** (13 new StartupChain tests,
+2 new AttestationModule tests, existing suites unchanged). TS-layer typecheck clean.
+
+## Contract fixes
+
+| # | Sev | Fix | Test |
+|---|-----|-----|------|
+| 1 | HIGH | **`recordCompany` caller auth.** The caller must be an owner of the target Safe (`ISafe.isOwner`), and the Safe must be deployed. Closes the `ensNameToCompanyId` squat / front-run vector (previously anyone could register any name/Safe). | `test_recordCompany_squatByNonOwnerReverts`, `test_recordCompany_nonContractSafeReverts` |
+| 2 | HIGH | **Admin → `Ownable2Step` + timelock.** Ownership is 2-step (`transferOwnership` → `acceptOwnership`); deploy the owner as a **2/3 Safe**. `setFeeRecipient` is now `proposeFeeRecipient` → (2-day timelock) → `executeFeeRecipient`. | `test_ownershipIsTwoStep`, `test_setFeeRecipientIsTimelocked`, `test_withdrawOnlyOwner` |
+| 3 | HIGH | **CEI + reentrancy in `recordCompany`.** All state is written before the fee `.call`, and the function is `nonReentrant` (the fee transfer previously executed before state writes). A reverting fee recipient reverts the whole tx and persists nothing. | `test_recordCompany_revertingFeeRecipientRevertsWholeTx`, `test_recordCompany_feePaidToTreasury` |
+| 4 | HIGH | **Harden `AttestationModule` before it ships.** `onlyCompanyMember` now enforces real membership via the StartupChain registry (`isFounder`); the six schema-config setters are owner-gated (were permissionless one-time, i.e. first-caller-pins). | `testOnlyCompanyMemberCanAttest`, `testSchemaSetterIsOwnerGated` |
+| 5 | MED | **Correct `.eth` namehash.** `transferENS` / `createSubdomain` / `revokeSubdomain` now parent labels under `namehash("eth")` instead of the ENS root (the old code treated the label as a TLD). Unblocks the dashboard subdomain / resolution issues (#43/#44/#33). | `test_namehash_isEthParentedNotRoot` |
+| 6 | MED | **Bounded founder loops + reverse index.** `MAX_FOUNDERS = 50` caps every founder loop; a `founder → companyId[]` index (`getCompaniesByFounder`) replaces the off-chain O(n) full-registry scan; `isFounder` backs the #4 membership check. | `test_maxFoundersEnforced`, `test_founderReverseIndex` |
+
+## Off-chain fixes
+
+| # | Sev | Fix | File |
+|---|-----|-----|------|
+| 7 | MED | **Bind payment to the registration.** `checkPaymentStatusAction` now requires the payment tx to originate from a founder wallet of the registration (`allowedFrom`), closing the cross-user / cross-registration replay by non-participants, plus an optional per-registration `expectedCommitment` (tx `input`) hook that binds one payment to one (user, ENS) once the client sends it. | `payment-actions.ts`, `actions.ts` |
+| 8 | MED | **De-risk the hot relayer key.** The treasury is no longer the signer address by default: `STARTUPCHAIN_TREASURY_ADDRESS` sets a distinct treasury (ideally a Safe), separating the least-privilege gas payer from fee custody. Falls back to the signer with a loud warning for dev. | `startupchain-client.ts` |
+
+**#7 residual (flagged, not fully closed):** true single-use (one payment ⇒ at most one registration)
+needs either a persisted consumed-tx set or moving the fee on-chain into `recordCompany`'s `msg.value`.
+This repo has no server-side store yet; the founder binding closes the replay-by-non-participant vector
+and the commitment hook closes cross-ENS reuse when the client adopts it. Tracked as follow-up.
+
+## Tradeoffs surfaced
+
+- **Solo companies get a 1/1 Safe.** `recordCompany` allows `threshold == founders.length == 1`. A 1/1
+  Safe has no multisig protection, but enforcing a 2-of-N floor would block legitimate solo founders. We
+  **allow it and document it** rather than enforce a floor. `test_soloFounderOneOfOneSafeAllowed`.
+- **Subdomain ENS ownership model.** The correct namehash (#5) is necessary but not sufficient: the Safe
+  owns the 2LD, so `createSubdomain`/`revokeSubdomain` require the StartupChain contract to be an approved
+  operator of the company node (`ensRegistry.setApprovalForAll(startupChain, true)` from the Safe) or the
+  subdomain logic to move into a Safe module. This PR fixes the node math and documents the operator
+  requirement; the module refactor is out of scope.
+- **Founder reverse-index is append-only.** `getCompaniesByFounder` may list a company a founder has since
+  left (after `updateFounders`); callers confirm current membership with `isFounder`. This avoids O(n²)
+  removal bookkeeping.
+
+## Not in this PR (item #9 — separate, larger PR)
+
+Moving the cap-table detail off contract storage to a permanent Arweave snapshot + an EAS
+`CompanyFormed`/`CapTableAttested` anchor (per the AxoloDAO ADR-001 storage-tier rule) is a larger
+refactor with its own parity + squat/replay regression tests. Left as a follow-up so this PR stays a
+focused, reviewable hardening set.
+
+## Deploy notes (Sepolia)
+
+Constructors changed: `StartupChain(ensRegistry, ensResolver, feeRecipient, initialOwner)` and
+`AttestationModule(eas, startupChainRegistry, initialOwner)`. Set `STARTUPCHAIN_OWNER` (the 2/3 Safe),
+`FEE_RECIPIENT` / `STARTUPCHAIN_TREASURY_ADDRESS` (a treasury distinct from the signer). External audit
+precedes any mainnet deploy.

src/app/(app)/dashboard/setup/actions.ts

@@ -200,9 +200,11 @@ export async function commitEnsRegistrationAction({
   const { totalWei } = await getEnsRegistrationCostAction(ensName, verificationYears, founders.length)
   console.log(LOG_PREFIX, 'Expected payment amount:', totalWei)
 
+  // SECURITY (#7): bind the payment to a founder wallet of this registration.
   const paymentStatus = await checkPaymentStatusAction({
     paymentTxHash,
     minValueWei: totalWei,
+    allowedFrom: founderStructs.map((f) => f.wallet),
   })
   console.log(LOG_PREFIX, 'Payment verification result:', paymentStatus)
 

src/app/(app)/dashboard/setup/payment-actions.ts

@@ -53,20 +53,36 @@ export async function verifyPrepaymentAction({
 }
 
 /**
- * Check if user has already sent a payment transaction to treasury
- * by looking at recent transactions (simplified approach)
+ * Verify a treasury prepayment transaction.
  *
- * SECURITY: This function verifies:
- * 1. Transaction exists and was successful
- * 2. Transaction was sent TO the treasury address
- * 3. Transaction value is >= minimum required (if specified)
+ * SECURITY: verifies that the transaction
+ * 1. exists and was successful,
+ * 2. was sent TO the treasury address,
+ * 3. has value >= the minimum required (if specified),
+ * 4. (#7 — payment binding) was sent FROM one of the registration's founder wallets, so a stranger's
+ *    payment can't be cited and a payment can't be replayed across registrations whose founder set does
+ *    not include the original payer, and
+ * 5. (optional) carries the expected per-registration commitment in its calldata (`expectedCommitment`),
+ *    which — once the client sends it — binds one payment to exactly one (user, ENS) registration.
+ *
+ * NOTE (#7 residual): true single-use (one payment ⇒ at most one registration) additionally requires
+ * either a persisted consumed-tx set or moving the fee on-chain into `recordCompany`'s msg.value. This
+ * repo has no server-side store yet; the founder binding above closes the cross-user/cross-registration
+ * replay vector, and `expectedCommitment` closes cross-ENS reuse when the client adopts it. Tracked as a
+ * follow-up.
  */
 export async function checkPaymentStatusAction({
   paymentTxHash,
   minValueWei,
+  allowedFrom,
+  expectedCommitment,
 }: {
   paymentTxHash: string
   minValueWei?: string
+  /** Founder wallet addresses; the payment must originate from one of them. */
+  allowedFrom?: string[]
+  /** 0x-hex the payment tx `input` must equal (per-registration commitment). Enforced only if set. */
+  expectedCommitment?: string
 }) {
   if (!paymentTxHash || !paymentTxHash.startsWith("0x")) {
     return { confirmed: false, error: "Invalid transaction hash" }
@@ -95,7 +111,29 @@ export async function checkPaymentStatusAction({
       if (tx.value < minValue) {
         return {
           confirmed: false,
-          error: `Insufficient payment: received ${tx.value.toString()}, required ${minValueWei}`
+          error: `Insufficient payment: received ${tx.value.toString()}, required ${minValueWei}`,
+        }
+      }
+    }
+
+    // SECURITY (#7): bind the payment to a founder of this registration.
+    if (allowedFrom && allowedFrom.length > 0) {
+      const from = tx.from?.toLowerCase()
+      const isFromFounder = allowedFrom.some((a) => a.toLowerCase() === from)
+      if (!isFromFounder) {
+        return {
+          confirmed: false,
+          error: "Payment must be sent from a founder wallet of this registration",
+        }
+      }
+    }
+
+    // SECURITY (#7, optional): bind the payment to this specific registration via a commitment.
+    if (expectedCommitment) {
+      if ((tx.input ?? "0x").toLowerCase() !== expectedCommitment.toLowerCase()) {
+        return {
+          confirmed: false,
+          error: "Payment commitment does not match this registration",
         }
       }
     }

src/contracts/foundry.lock

@@ -1,5 +1,11 @@
 {
   "lib/forge-std": {
     "rev": "8bbcf6e3f8f62f419e5429a0bd89331c85c37824"
+  },
+  "lib/openzeppelin-contracts": {
+    "tag": {
+      "name": "v5.1.0",
+      "rev": "69c8def5f222ff96f2b5beff05dfba996368aa79"
+    }
   }
 }

src/contracts/foundry.toml

@@ -2,6 +2,7 @@
 src = "src"
 out = "out"
 libs = ["lib"]
+solc = "0.8.28"
 optimizer = true
 optimizer_runs = 200
 via_ir = true

src/contracts/remappings.txt

@@ -0,0 +1,2 @@
+@openzeppelin/contracts/=lib/openzeppelin-contracts/contracts/
+forge-std/=lib/forge-std/src/

src/contracts/script/DeployAttestationModule.s.sol

@@ -32,7 +32,11 @@ contract DeployAttestationModule is Script {
         uint256 deployerPrivateKey = vm.envUint("DEPLOYER_KEY");
         address deployer = vm.addr(deployerPrivateKey);
 
+        // #4 — schema config is owner-gated; deploy the owner as a 2/3 Safe.
+        address initialOwner = vm.envOr("STARTUPCHAIN_OWNER", deployer);
+
         console.log("Deploying AttestationModule with deployer:", deployer);
+        console.log("Owner (should be a 2/3 Safe):", initialOwner);
         console.log("EAS address:", easAddress);
         console.log("StartupChain registry:", startupChainRegistry);
         console.log("Chain ID:", chainId);
@@ -41,7 +45,8 @@ contract DeployAttestationModule is Script {
 
         AttestationModule attestationModule = new AttestationModule(
             easAddress,
-            startupChainRegistry
+            startupChainRegistry,
+            initialOwner
         );
 
         console.log("AttestationModule deployed at:", address(attestationModule));

src/contracts/script/DeployStartupChain.s.sol

@@ -14,19 +14,23 @@ contract DeployStartupChain is Script {
         uint256 deployerPrivateKey = vm.envUint("DEPLOYER_KEY");
         address deployer = vm.addr(deployerPrivateKey);
 
-        // Fee recipient is the deployer (treasury)
-        address feeRecipient = deployer;
+        // #2/#8 — owner should be a 2/3 Safe; fee recipient (treasury) should be SEPARATE from the hot
+        // signer. Both default to the deployer only if not provided (loudly, for local dev).
+        address initialOwner = vm.envOr("STARTUPCHAIN_OWNER", deployer);
+        address feeRecipient = vm.envOr("FEE_RECIPIENT", deployer);
 
         console.log("Deploying StartupChain with deployer:", deployer);
-        console.log("Fee recipient:", feeRecipient);
+        console.log("Owner (should be a 2/3 Safe):", initialOwner);
+        console.log("Fee recipient (treasury):", feeRecipient);
         console.log("Chain ID:", block.chainid);
 
         vm.startBroadcast(deployerPrivateKey);
 
         StartupChain startupChain = new StartupChain(
             ensRegistry,
             ensResolver,
-            feeRecipient
+            feeRecipient,
+            initialOwner
         );
 
         console.log("StartupChain deployed at:", address(startupChain));

src/contracts/src/AttestationModule.sol

@@ -1,5 +1,12 @@
 // SPDX-License-Identifier: MIT
-pragma solidity ^0.8.13;
+pragma solidity 0.8.28;
+
+import {Ownable, Ownable2Step} from "@openzeppelin/contracts/access/Ownable2Step.sol";
+
+/// @notice The StartupChain registry membership check (#4 — replaces the no-op onlyCompanyMember).
+interface IStartupChainRegistry {
+    function isFounder(uint256 companyId, address account) external view returns (bool);
+}
 
 interface IEAS {
     struct AttestationRequest {
@@ -34,7 +41,11 @@ interface IEAS {
     function getAttestation(bytes32 uid) external view returns (Attestation memory);
 }
 
-contract AttestationModule {
+/// @title AttestationModule
+/// @notice EAS wrapper for company attestations. Hardened (#4): `onlyCompanyMember` now enforces real
+/// StartupChain-registry membership, and the schema-config setters are owner-gated (not permissionless),
+/// so the schema UIDs can no longer be front-run/pinned by the first caller. Admin = a 2/3 Safe.
+contract AttestationModule is Ownable2Step {
     enum AttestationType {
         CompanyFormation,
         GovernanceDecision,
@@ -90,48 +101,52 @@ contract AttestationModule {
     );
 
     modifier onlyCompanyMember(uint256 _companyId) {
-        // In production, this would check against StartupChain registry
-        // For now, we'll allow any address to attest (can be restricted later)
+        // #4 — real membership check against the StartupChain registry.
+        require(
+            IStartupChainRegistry(startupChainRegistry).isFounder(_companyId, msg.sender), "Not a company member"
+        );
         _;
     }
 
-    constructor(address _eas, address _startupChainRegistry) {
+    /// @param _initialOwner Admin for schema configuration — deploy as a 2/3 Safe multisig.
+    constructor(address _eas, address _startupChainRegistry, address _initialOwner) Ownable(_initialOwner) {
+        require(_startupChainRegistry != address(0), "Invalid registry");
         eas = IEAS(_eas);
         startupChainRegistry = _startupChainRegistry;
     }
 
-    // Schema management functions
-    function setCompanyFormationSchema(bytes32 _schema) external {
+    // Schema management functions — #4 owner-gated (was permissionless one-time).
+    function setCompanyFormationSchema(bytes32 _schema) external onlyOwner {
         require(companyFormationSchema == bytes32(0), "Schema already set");
         companyFormationSchema = _schema;
         emit SchemaRegistered(AttestationType.CompanyFormation, _schema);
     }
 
-    function setGovernanceDecisionSchema(bytes32 _schema) external {
+    function setGovernanceDecisionSchema(bytes32 _schema) external onlyOwner {
         require(governanceDecisionSchema == bytes32(0), "Schema already set");
         governanceDecisionSchema = _schema;
         emit SchemaRegistered(AttestationType.GovernanceDecision, _schema);
     }
 
-    function setFinancialTransactionSchema(bytes32 _schema) external {
+    function setFinancialTransactionSchema(bytes32 _schema) external onlyOwner {
         require(financialTransactionSchema == bytes32(0), "Schema already set");
         financialTransactionSchema = _schema;
         emit SchemaRegistered(AttestationType.FinancialTransaction, _schema);
     }
 
-    function setMilestoneAchievementSchema(bytes32 _schema) external {
+    function setMilestoneAchievementSchema(bytes32 _schema) external onlyOwner {
         require(milestoneAchievementSchema == bytes32(0), "Schema already set");
         milestoneAchievementSchema = _schema;
         emit SchemaRegistered(AttestationType.MilestoneAchievement, _schema);
     }
 
-    function setMembershipChangeSchema(bytes32 _schema) external {
+    function setMembershipChangeSchema(bytes32 _schema) external onlyOwner {
         require(membershipChangeSchema == bytes32(0), "Schema already set");
         membershipChangeSchema = _schema;
         emit SchemaRegistered(AttestationType.MembershipChange, _schema);
     }
 
-    function setContractDeploymentSchema(bytes32 _schema) external {
+    function setContractDeploymentSchema(bytes32 _schema) external onlyOwner {
         require(contractDeploymentSchema == bytes32(0), "Schema already set");
         contractDeploymentSchema = _schema;
         emit SchemaRegistered(AttestationType.ContractDeployment, _schema);