Skip to content

fix: validate release keys and smoke-run the genesis build on PRs #254

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
mordamax wants to merge 3 commits into from
Closed

fix: validate release keys and smoke-run the genesis build on PRs #254

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7b9dec2
fix: validate the genesis owner key, no mnemonic fallback
mordamax Aug 28, 2026
defb5ec
ci: smoke-run the genesis build on PRs with the release secrets
mordamax Aug 28, 2026
de691e8
ci: bun install before the smoke build, postinstall pins lib/ SHAs
mordamax Aug 28, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
82 changes: 82 additions & 0 deletions .github/workflows/genesis-build-smoke.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
name: Genesis Build Smoke

# Runs the release-time genesis build on PRs, with the same secrets and the same command,
# so a bad secret or a broken pipeline fails here instead of on master when a release is
# cut. The extractor's unit tests and `bash -n` cannot catch either — v0.5.6-rc1 died on a
# key `cast` could not decode, after every PR check had passed.
#
# Skipped on fork PRs: secrets are withheld there.

@re-gius re-gius Aug 31, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This hands a production key to any PR author, e.g. through a PR that edits build-genesis.sh to extract it. Moreover, this key is expensive to rotate since every DotNS address derives from its hash.
Instead, we could create a GitHub deployment environment (e.g., genesis-smoke) in this repository with a "Required reviewers" protection rule, so that only named reviewers can approve it, and move FACTORY_DEPLOYER_KEY to its environment secrets.
Otherwise, drop the pull_request trigger entirely and keep push: master

on:
pull_request:

@sphamjoli sphamjoli Aug 31, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The preflight's fine, keep it. It's the smoke workflow I'd drop, starting with this pull_request trigger.

The release already does this. publish-release.yml builds the genesis with these same secrets on the tag, so a bad key fails the release before anything publishes. That is how v0.5.6-rc1 surfaced. The smoke runs the same check earlier, at PR time.

Moving it to PR time has a downside worth weighing. This job runs build-genesis.sh with DOTNS_ADMIN_KEY and FACTORY_DEPLOYER_KEY in env, and the paths filter includes the scripts and contracts a PR edits. So any same-repo PR can change the script (or, with ffi = true, any executed .sol) to print the keys, and the job runs it before review. The if: guards forks, not a write-access member or a leaked token. DOTNS_ADMIN_KEY owns the registry, resolvers, registrar and beacons, and cannot be rotated out of a shipped genesis.

And this is what dotns-releases is for. It's the private repo that owns deploy and release, and these credentials already live there behind its access controls. Putting the same production keys into the public contracts repo's PR CI widens where they live and hands them to PR-authored code from any contributor. A keyed rehearsal belongs there, run deliberately, not on every PR here.

The fix the incident calls for is the preflight, not a new workflow: it turns the bare decode error into a readable one, and dropping the mnemonic fallback removes the which-key ambiguity. I'd land that half, ideally as the first step of the publish workflows so a bad key fails fast, and drop the smoke.

@mordamax mordamax Aug 31, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

feel free to drop this PR, as i mentioned in matrix - it's optional

branches: [master]
paths:
- "scripts/genesis/**"
- "scripts/deploy/**"
- "contracts/**"
- "**.sol"
- "deployments/**"
- "foundry.toml"
- ".github/workflows/publish-release.yml"
- ".github/workflows/publish-prerelease.yml"
- ".github/workflows/genesis-build-smoke.yml"
- ".github/actions/setup-foundry/action.yml"
push:
branches: [master]
paths:
- "scripts/genesis/**"

@re-gius re-gius Aug 31, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should add some others here:

  • "scripts/deploy/**" the stages the genesis is built from
  • "deployments/**" what the check asserts against
  • "bun.lock" since it may break the build
  • "package.json" updates may break the build
  • ".github/workflows/publish-release.yml"
  • ".github/workflows/publish-prerelease.yml"

- ".github/workflows/genesis-build-smoke.yml"
workflow_dispatch:

permissions:
contents: read

concurrency:
group: genesis-smoke-${{ github.event.pull_request.number || github.ref }}
cancel-in-progress: true

jobs:
genesis-build-smoke:
if: github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository
runs-on: ubuntu-latest

@re-gius re-gius Aug 31, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: add timeout-minutes: 45 or similar

@re-gius re-gius Aug 31, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moreover, we should add

env:
  DOTNS_TLD: testnet

as from #255 and always reference ${{ env.DOTNS_TLD }} below

steps:
- uses: actions/checkout@v4
with:
submodules: recursive

- uses: ./.github/actions/setup-foundry

- uses: actions/setup-node@v4
with:
node-version: "20"

- uses: oven-sh/setup-bun@v2
with:
bun-version: "1.2.6"
no-cache: true

# Not optional: bun's postinstall runs setup.bash, which lands lib/ dependencies on
# their pinned SHAs. Without it the build compiles against stale OpenZeppelin and
# fails on override/mutability mismatches — the release workflow always runs this.
- run: bun install

# The same secrets and command as the release step. The parity check inside the
# script runs for real: with the real factory key the addresses must equal the
# committed manifest, so nothing is skipped or stubbed.
- name: Build the genesis exactly as a release does
env:
FOUNDRY_DISABLE_NIGHTLY_WARNING: "1"
FACTORY_DEPLOYER_KEY: ${{ secrets.FACTORY_DEPLOYER_KEY }}
DOTNS_ADMIN_KEY: ${{ secrets.DOTNS_ADMIN_KEY }}
DOTNS_TLD: test
run: bash scripts/genesis/build-genesis.sh release

- name: Assert the artifact shape
run: |
test -s release/dotns-genesis-test.json
jq -e '.tld == "test" and (.accounts | length > 0)' release/dotns-genesis-test.json >/dev/null
echo "accounts: $(jq '.accounts | length' release/dotns-genesis-test.json)"

- uses: actions/upload-artifact@v4
with:
name: dotns-genesis-test
path: release/dotns-genesis-test.json
1 change: 0 additions & 1 deletion .github/workflows/publish-prerelease.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,6 @@ jobs:
FOUNDRY_DISABLE_NIGHTLY_WARNING: "1"
FACTORY_DEPLOYER_KEY: ${{ secrets.FACTORY_DEPLOYER_KEY }}
DOTNS_ADMIN_KEY: ${{ secrets.DOTNS_ADMIN_KEY }}
DOTNS_ADMIN_MNEMONIC: ${{ secrets.DOTNS_ADMIN_MNEMONIC }}
run: bash scripts/genesis/build-genesis.sh release

- name: Extract ABIs
Expand Down
6 changes: 2 additions & 4 deletions .github/workflows/publish-release.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,15 +113,13 @@ jobs:
# contracts, the deploy scripts and the CREATE3 factory key that fixes every address,
# and the artifact then ships from the same commit as the ABIs beside it.
#
# The owner key is taken from DOTNS_ADMIN_KEY if set, otherwise derived from
# DOTNS_ADMIN_MNEMONIC. It ends up owning the registry, resolvers, registrar, store factory
# and beacons in the genesis storage, so the script refuses to run without one.
# DOTNS_ADMIN_KEY (a raw private key, the only accepted credential) ends up owning the
# registry, resolvers, registrar, store factory and beacons in the genesis storage.
- name: Build pallet-revive genesis
env:
FOUNDRY_DISABLE_NIGHTLY_WARNING: "1"
FACTORY_DEPLOYER_KEY: ${{ secrets.FACTORY_DEPLOYER_KEY }}
DOTNS_ADMIN_KEY: ${{ secrets.DOTNS_ADMIN_KEY }}
DOTNS_ADMIN_MNEMONIC: ${{ secrets.DOTNS_ADMIN_MNEMONIC }}
run: bash scripts/genesis/build-genesis.sh release

- name: Extract ABIs
Expand Down
61 changes: 36 additions & 25 deletions scripts/genesis/build-genesis.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,23 +37,16 @@ SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
DEPLOYMENT_FILE="deployments/localhost/31337.json"
CANONICAL_MANIFEST="deployments/paseo-assethub/420420417.json"

# Who OWNS the contracts in the genesis state (REQUIRED, one of the three below).
# Who OWNS the contracts in the genesis state: DOTNS_ADMIN_KEY, a raw private key,
# REQUIRED. No DotNS *address* depends on it — with CREATE3 the addresses are a pure
# function of the factory — but every ownership and role assignment written into
# genesis storage does.
#
# No DotNS *address* depends on this key — with CREATE3 the addresses are a pure
# function of the factory (see FACTORY_DEPLOYER_KEY below, which owns only the
# factory) — but every ownership and role assignment written into genesis storage
# does: this key ends up owning the registry, the resolvers, the registrar, the
# store factory and the beacons.
#
# Accepted, in order of precedence:
# DOTNS_ADMIN_KEY a raw private key — the admin credential this repo already holds
# DOTNS_ADMIN_MNEMONIC the admin mnemonic; index $DOTNS_ADMIN_INDEX (default 0)
#
# Deliberately NOT accepted: DOTNS_MNEMONIC. That is the operational credential the
# whitelist workflows drive the `dotns` CLI with, not the contract admin, and quietly
# making it the owner of every contract in a genesis would be a hard mistake to spot.
# Not DEPLOYER_KEY: that name is dotns-releases' own secret, and accepting it here
# would make which key owns a published genesis depend on which repo the build ran in.
# The ONLY accepted credential. No mnemonic, no fallback: with two credentials the
# build would guess which account owns everything, and nothing downstream would
# notice a wrong guess — the parity check validates addresses, not owners.
# Not DOTNS_MNEMONIC (the whitelist workflows' operational credential) and not
# DEPLOYER_KEY (dotns-releases' own secret) for the same reason.

@re-gius re-gius Aug 31, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: why the same reason? I don't get it

ADMIN_KEY="${DOTNS_ADMIN_KEY:-}"

# Single-purpose CREATE3 factory deployer key (REQUIRED). Every DotNS address is
Expand Down Expand Up @@ -90,18 +83,12 @@ for tool in forge anvil cast node jq curl; do
command -v "$tool" >/dev/null 2>&1 || { echo "Error: $tool is not on PATH" >&2; exit 1; }
done

# Needs cast, so it happens after the check above.
if [ -z "$ADMIN_KEY" ] && [ -n "${DOTNS_ADMIN_MNEMONIC:-}" ]; then
ADMIN_KEY="$(cast wallet private-key --mnemonic "$DOTNS_ADMIN_MNEMONIC" "${DOTNS_ADMIN_INDEX:-0}")"
echo "Owner key derived from DOTNS_ADMIN_MNEMONIC, index ${DOTNS_ADMIN_INDEX:-0}."
fi

if [ -z "$ADMIN_KEY" ]; then
cat >&2 <<'MSG'
Error: no owner key. Set DOTNS_ADMIN_KEY or DOTNS_ADMIN_MNEMONIC.
Error: DOTNS_ADMIN_KEY is required.

Whichever is given becomes the owner of every DotNS contract in the genesis
state, so this build refuses to fall back to a public dev key.
It becomes the owner of every DotNS contract in the genesis state, so this
build refuses to fall back to a public dev key or any other credential.
MSG
exit 1
fi
Expand All @@ -116,6 +103,30 @@ MSG
exit 1
fi

# ---- Key shape preflight ----
# `cast` rejects a malformed key with a bare "Failed to decode private key" — no variable
# name, no shape — and a GitHub secret cannot be read back to inspect. Describe the problem
# (length, word count) without ever printing the value. Surrounding whitespace is stripped:

@re-gius re-gius Aug 31, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: ALL whitespace is stripped, not just surrounding

# a pasted trailing newline is the classic way a secret breaks.
require_hex_key() {
local name="$1" raw="${2-}"
if printf '%s' "$raw" | tr -d '[:space:]' | grep -Eq '^(0x)?[0-9a-fA-F]{64}$'; then
return 0
fi
local words; words=$(printf '%s' "$raw" | wc -w | tr -d ' ')
echo "Error: $name is not a raw private key (need 64 hex chars, 0x optional):" >&2
echo " got ${#raw} char(s), $words word(s)." >&2
if [ "$words" -ge 12 ]; then
echo " That shape is a mnemonic. This build takes only a key — derive one with" >&2
echo " \`cast wallet private-key --mnemonic '<...>'\` and store the result." >&2
fi
exit 1
}
require_hex_key DOTNS_ADMIN_KEY "$ADMIN_KEY"
ADMIN_KEY="$(printf '%s' "$ADMIN_KEY" | tr -d '[:space:]')"
require_hex_key FACTORY_DEPLOYER_KEY "$FACTORY_DEPLOYER_KEY"
FACTORY_DEPLOYER_KEY="$(printf '%s' "$FACTORY_DEPLOYER_KEY" | tr -d '[:space:]')"

DEPLOYER_ADDR="$(cast wallet address --private-key "$ADMIN_KEY")"
export WHITELIST_OPERATOR="${WHITELIST_OPERATOR:-$DEPLOYER_ADDR}"
echo "Contract owner: $DEPLOYER_ADDR"
Expand Down
Loading