diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS new file mode 100644 index 00000000..1b4a3205 --- /dev/null +++ b/.github/CODEOWNERS @@ -0,0 +1,27 @@ +# CODEOWNERS — required reviewers for security-critical surfaces. +# +# CPL-291: changes to on-chain contract code and to CI/CD workflows must not +# land on a single self-approval. A PR touching any path below requires review +# from the listed owner(s). +# +# IMPORTANT — this file only takes effect once branch protection on the default +# branch enables "Require review from Code Owners". Configure that in +# Settings → Branches. Note this gates the DEFAULT branch only: the `next` +# branch that upgrade-next deploys from gets no CODEOWNERS coverage unless it +# also receives branch protection (acceptable for staging). +# +# TODO(CPL-291 follow-up): replace the individual owner with a proper GitHub +# team slug (e.g. @LIT-Protocol/contracts, @LIT-Protocol/platform-security) +# once the teams exist. An unknown team here is silently ignored by GitHub, so +# a real, verified handle is used in the meantime to keep the rule enforceable. + +# --- CI / CD pipeline ------------------------------------------------------- +/.github/ @GTC6244 +/.github/workflows/ @GTC6244 +/.github/CODEOWNERS @GTC6244 + +# --- On-chain Diamond contract code + deployer ------------------------------ +/lit-api-server/blockchain/ @GTC6244 + +# Node config files drive which network/Diamond a deploy targets. +/lit-api-server/NodeConfig.*.toml @GTC6244 diff --git a/.github/workflows/manual_contract-upgrade-main-2-verify.yml b/.github/workflows/manual_contract-upgrade-main-2-verify.yml new file mode 100644 index 00000000..436b6fdd --- /dev/null +++ b/.github/workflows/manual_contract-upgrade-main-2-verify.yml @@ -0,0 +1,121 @@ +# Contract Upgrade Main — Phase 2: Verify +# +# Run this AFTER Safe multisig signers have approved and executed the +# diamondCut transaction proposed by the `main` branch of +# manual_contract-upgrade.yml (the "propose-main" job). +# +# This workflow: +# 1. Verifies the Safe transaction was executed on-chain +# 2. Compiles contracts from the current source code +# 3. Compares on-chain facet bytecode and selectors against compiled artifacts +# +# Network and contract address are read from: +# lit-api-server/NodeConfig.main.toml +# +# Required secrets: +# BASE_CHAIN_RPC - RPC endpoint for Base + +name: "Contract Upgrade Main 2: Verify" + +permissions: + contents: read + +on: + workflow_dispatch: + inputs: + safe_tx_hash: + description: "Safe transaction hash from the propose-main job" + required: true + type: string + +concurrency: + group: contract-upgrade-main + cancel-in-progress: false + +jobs: + verify-safe-tx: + runs-on: self-hosted + steps: + - uses: actions/checkout@v4 + with: + ref: main + + - uses: actions/setup-node@v4 + with: + node-version: 22 + + - name: Install contract dependencies + working-directory: lit-api-server/blockchain/lit_node_express + run: npm ci + + - name: Compile contracts + working-directory: lit-api-server/blockchain/lit_node_express + run: | + npx hardhat clean + npx hardhat compile + + - name: Verify Safe transaction was executed + id: safe + working-directory: lit-api-server/blockchain/lit_node_express + env: + BASE_RPC_URL: ${{ secrets.BASE_CHAIN_RPC }} + # Pass dispatcher-controlled input via env: rather than interpolating + # it into the shell script, and validate its shape first. + SAFE_TX_HASH: ${{ inputs.safe_tx_hash }} + SAFE_ADDRESS: ${{ vars.SAFE_ADDRESS_CHIPOTLE_MAIN }} + run: | + if ! printf '%s' "$SAFE_TX_HASH" | grep -Eq '^0x[a-fA-F0-9]{64}$'; then + echo "::error::Invalid safe_tx_hash '$SAFE_TX_HASH' (expected 0x + 64 hex chars)." + exit 1 + fi + if [ -z "$SAFE_ADDRESS" ]; then + echo "::error::vars.SAFE_ADDRESS_CHIPOTLE_MAIN is not configured — cannot bind the tx to the expected Safe." + exit 1 + fi + # --safe pins verification to our Safe: execute-safe-tx asserts the tx + # actually belongs to it, so an executed hash from any other Safe on + # Base cannot pass this gate. + OUTPUT=$(npx hardhat execute-safe-tx \ + --network base \ + --safe "$SAFE_ADDRESS" \ + --safe-tx-hash "$SAFE_TX_HASH" \ + 2>&1 | tee /dev/stderr) + TX_HASH=$(echo "$OUTPUT" | grep '^TX_HASH=' | cut -d= -f2) + if [ -z "$TX_HASH" ]; then + echo "::error::Safe transaction has not been executed yet." + exit 1 + fi + echo "tx_hash=$TX_HASH" >> "$GITHUB_OUTPUT" + + - name: Read contract address from NodeConfig + id: config + run: | + CONFIG="lit-api-server/NodeConfig.main.toml" + if [ ! -f "$CONFIG" ]; then + echo "::error::$CONFIG not found." + exit 1 + fi + ADDRESS=$(grep '^contract_address' "$CONFIG" | head -1 | sed 's/.*= *"\(.*\)"/\1/') + if [ -z "$ADDRESS" ]; then + echo "::error::contract_address not found in $CONFIG — cannot verify facets." + exit 1 + fi + echo "address=$ADDRESS" >> "$GITHUB_OUTPUT" + + - name: Verify diamond facets match source code + working-directory: lit-api-server/blockchain/lit_node_express + env: + BASE_RPC_URL: ${{ secrets.BASE_CHAIN_RPC }} + run: | + npx hardhat verify-diamond-facets \ + --network base \ + --diamond "${{ steps.config.outputs.address }}" + + - name: Summary + if: always() + run: | + echo "## Contract Upgrade Verification (main)" >> "$GITHUB_STEP_SUMMARY" + echo "" >> "$GITHUB_STEP_SUMMARY" + echo "**Safe TX Hash:** \`${{ inputs.safe_tx_hash }}\`" >> "$GITHUB_STEP_SUMMARY" + echo "**On-chain TX:** \`${{ steps.safe.outputs.tx_hash }}\`" >> "$GITHUB_STEP_SUMMARY" + echo "**Diamond:** \`${{ steps.config.outputs.address }}\`" >> "$GITHUB_STEP_SUMMARY" diff --git a/.github/workflows/manual_contract-upgrade.yml b/.github/workflows/manual_contract-upgrade.yml index c84a6b7f..aa3ab66f 100644 --- a/.github/workflows/manual_contract-upgrade.yml +++ b/.github/workflows/manual_contract-upgrade.yml @@ -2,12 +2,31 @@ # # Network and contract address are always read from the NodeConfig file # for the selected branch: -# main → lit-api-server/NodeConfig.main.toml -# next → lit-api-server/NodeConfig.next.toml +# main → lit-api-server/NodeConfig.main.toml (Base mainnet — production-critical) +# next → lit-api-server/NodeConfig.next.toml (Base — testing/staging Diamond) +# +# Branch behaviour (CPL-291 hardening): +# main → NEVER signed directly. Facets are deployed and a diamondCut is +# PROPOSED to the Safe multisig (vars.SAFE_ADDRESS_CHIPOTLE_MAIN). +# Safe signers must approve + execute, then run +# "Contract Upgrade Main 2: Verify". Gated by the `production-base` +# GitHub environment (required reviewers). +# next → direct `--action=update` against the testing Diamond, gated by the +# `staging-base` GitHub environment (light protection). +# +# REQUIRED GitHub environments (Settings → Environments), each with reviewers +# configured to taste: +# production-base - required reviewers (multi-person approval) for `main` +# staging-base - light gate for `next` # # Required secrets: -# CONTRACT_DEPLOYER_SECRET_SEPOLIA - deployer wallet private key (Base Sepolia) -# PHALA_DSTACKAPP_PRIVATE_KEY - deployer wallet private key (Base Mainnet) +# SAFE_PROPOSER_PRIVATE_KEY - EOA used to deploy facets + propose Safe txs (main) +# CONTRACT_DEPLOYER_SECRET_SEPOLIA - deployer wallet private key (non-base networks, next) +# PHALA_DSTACKAPP_PRIVATE_KEY - deployer wallet private key (Base, next) +# BASE_CHAIN_RPC - RPC endpoint for Base +# +# Required vars: +# SAFE_ADDRESS_CHIPOTLE_MAIN - Safe multisig that owns the main Diamond name: Contract Upgrade @@ -31,17 +50,141 @@ concurrency: cancel-in-progress: false jobs: - contract-upgrade: + # --------------------------------------------------------------------------- + # main → Base mainnet production Diamond. Safe-routed propose flow only. + # The Diamond owner/upgrade authority is never exposed to this job: the + # diamondCut is only *proposed* to the Safe multisig, whose signers must + # approve and execute it. A proposer EOA (SAFE_PROPOSER_PRIVATE_KEY) is used + # to deploy facets and sign the Safe proposal, but that key cannot upgrade + # the Diamond on its own. + # --------------------------------------------------------------------------- + propose-main: + if: ${{ inputs.branch == 'main' }} + runs-on: self-hosted + environment: production-base + steps: + - uses: actions/checkout@v4 + with: + ref: main + + - name: Read config from NodeConfig + id: config + run: | + CONFIG="lit-api-server/NodeConfig.main.toml" + + if [ ! -f "$CONFIG" ]; then + echo "::error::$CONFIG not found." + exit 1 + fi + + NETWORK=$(grep '^name' "$CONFIG" | head -1 | sed 's/.*= *"\(.*\)"/\1/') + ADDRESS=$(grep '^contract_address' "$CONFIG" | head -1 | sed 's/.*= *"\(.*\)"/\1/') + + if [ "$NETWORK" != "base" ]; then + echo "::error::main NodeConfig must target the 'base' network (got '$NETWORK')." + exit 1 + fi + # ADDRESS comes from a checked-out file a PR can mutate and is fed to + # later shell steps — reject anything that isn't a plain EVM address + # so a crafted NodeConfig cannot inject shell (CPL-293, PR #485). + if ! printf '%s' "$ADDRESS" | grep -Eq '^0x[a-fA-F0-9]{40}$'; then + echo "::error::Invalid contract_address '$ADDRESS' in $CONFIG (expected 0x + 40 hex chars)." + exit 1 + fi + if [ -z "${{ vars.SAFE_ADDRESS_CHIPOTLE_MAIN }}" ]; then + echo "::error::vars.SAFE_ADDRESS_CHIPOTLE_MAIN is not configured — cannot Safe-route the upgrade." + exit 1 + fi + + echo "network=$NETWORK" >> "$GITHUB_OUTPUT" + echo "address=$ADDRESS" >> "$GITHUB_OUTPUT" + echo "Resolved from $CONFIG: network=$NETWORK address=$ADDRESS" + + - uses: dtolnay/rust-toolchain@1.91 + + - uses: actions/setup-node@v4 + with: + node-version: 22 + + - name: Install contract dependencies + working-directory: lit-api-server/blockchain/lit_node_express + run: npm ci + + - name: Compile contracts + working-directory: lit-api-server/blockchain/lit_node_express + run: | + npx hardhat clean + npx hardhat compile + node generate-diamond-abi.mjs + + - name: Build contract deployer + working-directory: lit-api-server/blockchain/rust_generator_and_deployer + run: cargo build --locked --bin contract_deployer + + - name: Deploy facets and generate proposal + working-directory: lit-api-server/blockchain/lit_node_express + env: + DEPLOYER_KEY: ${{ secrets.SAFE_PROPOSER_PRIVATE_KEY }} + NETWORK: ${{ steps.config.outputs.network }} + ADDRESS: ${{ steps.config.outputs.address }} + RPC_URL: ${{ secrets.BASE_CHAIN_RPC }} + run: | + if [ -z "$DEPLOYER_KEY" ]; then + echo "::error::secrets.SAFE_PROPOSER_PRIVATE_KEY is not configured — refusing to fall back to the deployer's built-in default key." + exit 1 + fi + ../rust_generator_and_deployer/target/debug/contract_deployer \ + --action=propose-update \ + --network="$NETWORK" \ + --abifolder=artifacts/contracts \ + --secret="$DEPLOYER_KEY" \ + --address="$ADDRESS" \ + --rpc-url="$RPC_URL" \ + --output=diamond_cut_proposal.json + + - name: Propose diamondCut to Safe multisig + working-directory: lit-api-server/blockchain/lit_node_express + env: + PROPOSER_PRIVATE_KEY: ${{ secrets.SAFE_PROPOSER_PRIVATE_KEY }} + BASE_RPC_URL: ${{ secrets.BASE_CHAIN_RPC }} + SAFE_ADDRESS: ${{ vars.SAFE_ADDRESS_CHIPOTLE_MAIN }} + ADDRESS: ${{ steps.config.outputs.address }} + run: | + OUTPUT=$(npx hardhat propose-diamond-cut \ + --safe "$SAFE_ADDRESS" \ + --proposal diamond_cut_proposal.json \ + --network base \ + 2>&1 | tee /dev/stderr) + SAFE_TX_HASH=$(echo "$OUTPUT" | grep '^SAFE_TX_HASH=' | tail -1 | cut -d= -f2) + if [ -z "$SAFE_TX_HASH" ]; then + echo "::error::Could not extract SAFE_TX_HASH from propose-diamond-cut output — proposal may have failed." + exit 1 + fi + echo "## Contract Upgrade Proposal (main)" >> "$GITHUB_STEP_SUMMARY" + echo "" >> "$GITHUB_STEP_SUMMARY" + echo "**Diamond:** \`$ADDRESS\`" >> "$GITHUB_STEP_SUMMARY" + echo "**Safe TX Hash:** \`$SAFE_TX_HASH\`" >> "$GITHUB_STEP_SUMMARY" + echo "" >> "$GITHUB_STEP_SUMMARY" + echo "Signers can review and approve in the [Safe UI](https://app.safe.global/transactions/queue?safe=base:$SAFE_ADDRESS)." >> "$GITHUB_STEP_SUMMARY" + echo "" >> "$GITHUB_STEP_SUMMARY" + echo "After execution, run **Contract Upgrade Main 2: Verify** with this Safe TX Hash." >> "$GITHUB_STEP_SUMMARY" + + # --------------------------------------------------------------------------- + # next → testing/staging Diamond. Direct upgrade behind a light environment gate. + # --------------------------------------------------------------------------- + upgrade-next: + if: ${{ inputs.branch == 'next' }} runs-on: self-hosted + environment: staging-base steps: - uses: actions/checkout@v4 with: - ref: ${{ inputs.branch }} + ref: next - name: Read config from NodeConfig id: config run: | - CONFIG="lit-api-server/NodeConfig.${{ inputs.branch }}.toml" + CONFIG="lit-api-server/NodeConfig.next.toml" if [ ! -f "$CONFIG" ]; then echo "::error::$CONFIG not found." @@ -51,6 +194,22 @@ jobs: NETWORK=$(grep '^name' "$CONFIG" | head -1 | sed 's/.*= *"\(.*\)"/\1/') ADDRESS=$(grep '^contract_address' "$CONFIG" | head -1 | sed 's/.*= *"\(.*\)"/\1/') + # NETWORK/ADDRESS come from a checked-out file the `next` branch can + # mutate and are consumed by later shell steps — validate them against + # the deployer's accepted networks / a plain EVM address so a crafted + # NodeConfig cannot inject shell (CPL-293, PR #485). + case "$NETWORK" in + anvil|yellowstone|base-sepolia|base) ;; + *) + echo "::error::Invalid network '$NETWORK' in $CONFIG (expected anvil, yellowstone, base-sepolia, or base)." + exit 1 + ;; + esac + if ! printf '%s' "$ADDRESS" | grep -Eq '^0x[a-fA-F0-9]{40}$'; then + echo "::error::Invalid contract_address '$ADDRESS' in $CONFIG (expected 0x + 40 hex chars)." + exit 1 + fi + echo "network=$NETWORK" >> "$GITHUB_OUTPUT" echo "address=$ADDRESS" >> "$GITHUB_OUTPUT" echo "Resolved from $CONFIG: network=$NETWORK address=$ADDRESS" @@ -80,11 +239,21 @@ jobs: working-directory: lit-api-server/blockchain/lit_node_express env: DEPLOYER_KEY: ${{ steps.config.outputs.network == 'base' && secrets.PHALA_DSTACKAPP_PRIVATE_KEY || (steps.config.outputs.network != 'base' && secrets.CONTRACT_DEPLOYER_SECRET_SEPOLIA) }} + NETWORK: ${{ steps.config.outputs.network }} + ADDRESS: ${{ steps.config.outputs.address }} + RPC_URL: ${{ secrets.BASE_CHAIN_RPC }} run: | + # The DEPLOYER_KEY ternary yields "" (base, key unset) or the literal + # "false" (non-base, key unset); reject both so we never fall back to + # contract_deployer's built-in default key. + if [ -z "$DEPLOYER_KEY" ] || [ "$DEPLOYER_KEY" = "false" ]; then + echo "::error::Deployer key for network '$NETWORK' is not configured (PHALA_DSTACKAPP_PRIVATE_KEY / CONTRACT_DEPLOYER_SECRET_SEPOLIA)." + exit 1 + fi ../rust_generator_and_deployer/target/debug/contract_deployer \ --action=update \ - --network=${{ steps.config.outputs.network }} \ + --network="$NETWORK" \ --abifolder=artifacts/contracts \ --secret="$DEPLOYER_KEY" \ - --address=${{ steps.config.outputs.address }} \ - --rpc-url=${{ secrets.BASE_CHAIN_RPC }} + --address="$ADDRESS" \ + --rpc-url="$RPC_URL" diff --git a/lit-api-server/blockchain/lit_node_express/tasks/execute-safe-tx.ts b/lit-api-server/blockchain/lit_node_express/tasks/execute-safe-tx.ts index 68c3eb83..424f454b 100644 --- a/lit-api-server/blockchain/lit_node_express/tasks/execute-safe-tx.ts +++ b/lit-api-server/blockchain/lit_node_express/tasks/execute-safe-tx.ts @@ -21,6 +21,20 @@ task("execute-safe-tx", "Verify a Safe transaction has been executed and return const safeAddress = taskArgs.safe || safeTransaction.safe; console.log(`Safe: ${safeAddress}`); + // When an expected Safe is supplied, assert the tx actually belongs to it. + // Otherwise an executed tx hash from *any* Safe on this chain would pass + // verification and get reported as the on-chain upgrade tx. + if ( + taskArgs.safe && + safeTransaction.safe.toLowerCase() !== taskArgs.safe.toLowerCase() + ) { + console.error( + `\nSafe transaction ${safeTxHash} belongs to Safe ${safeTransaction.safe}, ` + + `not the expected ${taskArgs.safe}.` + ); + process.exit(1); + } + if (!safeTransaction.isExecuted || !safeTransaction.transactionHash) { console.error( `\nSafe transaction has not been executed yet. ` + @@ -32,7 +46,7 @@ task("execute-safe-tx", "Verify a Safe transaction has been executed and return if (safeTransaction.isSuccessful === false) { console.error( `\nSafe transaction was executed but REVERTED on-chain (tx: ${safeTransaction.transactionHash}). ` + - `The addComposeHash call did not succeed. Check the transaction on Basescan.` + `The operation did not succeed. Check the transaction on Basescan.` ); process.exit(1); }