Skip to content

feat: detect leaked private keys by deriving them, not matching them - #69

Closed
maciejwitowski wants to merge 4 commits into
mainfrom
maciej/keyscan-private-key-detection
Closed

maciejwitowski wants to merge 4 commits into
mainfrom
maciej/keyscan-private-key-detection

Conversation

@maciejwitowski

Copy link
Copy Markdown
Contributor

Adds an advisory CI check that looks for leaked EOA private keys in a pull request diff.

Why a regex does not work here

An Ethereum private key has no structure — 32 uniformly random bytes, no checksum, no version byte, no prefix. The only available pattern is (0x)?[0-9a-f]{64}, and in this repository that also matches transaction hashes, block hashes, every keccak digest, merkle and state roots, storage slots, ABI-encoded 32-byte words, and validator deposit_data fields. 32-byte hex is the native unit of this codebase.

This is why GitHub has no built-in detector for private keys, and why the off-the-shelf tools produce more noise than signal on a contracts repo.

What this does instead

Derive the address each candidate would control, then ask the chain whether that address has ever been used. A digest derives to an address with no nonce and no balance; a real key does not. Generic scanners cannot do this because they have no chain to ask — we do.

Measured over the tracked tree:

Stage Count
64-hex candidates 5,782
Distinct values 588
Surviving the filters 2

Both survivors were triaged by hand; neither was a false positive.

The three rules that keep it quiet

Each is a statement about entropy rather than a denylist that has to be maintained:

  1. Entropy bounds — a key must lie in [2^128, N). Anything smaller is an ABI-encoded integer (bytes32(1), a length prefix, an array offset), and those derive to the well-known "private key = N" accounts that have been used on every chain.
  2. Public values — the EIP-1967 proxy slots and the Hardhat/Anvil development accounts are valid keys belonging to nobody, and being public they have genuine on-chain history. The implementation slot appears in every proxy deployment file we publish and those accounts appear in every fixture, so without this rule they alone produce hundreds of findings. All computed from their definitions, so the list cannot drift.
  3. Breadth downgrade — a value in three or more files is a shared constant, not a leak. Breadth never opens a finding, only lowers the severity of one already raised.

Deliberate choices

  • Advisory, not blocking. continue-on-error: true and no --fail-on-verified, so the real false-positive rate can be observed before anyone's merge depends on it. Promoting it later is a two-line change, documented in the README.
  • Diff-scoped. Costs nothing, so the same check can be adopted on private repos where GitHub's paid secret scanning is not enabled.
  • Fails loud, not quiet. A rate-limited or unreachable RPC is reported as incomplete rather than silently treated as clean.
  • Never prints a key in full. Output is redacted to 0xabcdef…1234 plus the derived address — enough to locate the line without repeating the secret into CI logs.
  • --ignore-scripts on install. A secret scanner is the last place to be running package postinstall hooks.
  • Custom patterns were not an option. GitHub secret scanning custom patterns require Secret Protection on an org-owned repo, and would still be matching on shape.

Known blind spot

A freshly generated key that was never funded derives to an address with no history, so liveness will not see it. The optional known-address register and the secret-shaped-context check cover part of that tail, not all of it. This is a backstop — it does not replace keeping production keys somewhere they cannot be committed.

Verification

  • 22 unit tests, no network and no chain required: npx mocha --require ts-node/register scripts/keyscan/scan.test.ts
  • Both CI commands run end-to-end locally against this repo's dependency tree.
  • Full-tree and diff modes exercised against Vana mainnet and Moksha.

🤖 Generated with Claude Code

@maciejwitowski
maciejwitowski force-pushed the maciej/keyscan-private-key-detection branch 3 times, most recently from ba35c6a to b8ff1e8 Compare August 11, 2026 09:25
An Ethereum private key has no structure: 32 uniformly random bytes, no
checksum, no version byte, no prefix. The only pattern available is
`(0x)?[0-9a-f]{64}`, which in this repository also matches transaction
hashes, keccak digests, merkle and state roots, storage slots,
ABI-encoded words and validator deposit_data fields. 32-byte hex is the
native unit of this codebase, so pattern-based scanners are unusable
here — which is why GitHub ships no detector for this type.

This derives the address each candidate would control and asks the chain
whether it has ever been used. Digests derive to addresses with no
history; a real key does not. Generic scanners cannot do this because
they have no chain to ask.

Measured over the tracked tree: 5,782 candidates, 588 distinct, 2
surviving the filters, no false positives among them.

Three rules keep the noise out, each a statement about entropy rather
than a denylist needing maintenance:

- Entropy bounds. A key must lie in [2^128, N). Anything smaller is an
  ABI-encoded integer, and those derive to the well-known "private
  key = N" accounts that have been used on every chain.
- Public values. The EIP-1967 proxy slots and the Hardhat/Anvil
  development accounts are valid keys owned by nobody, and being public
  they have real on-chain history. The proxy slot is in every proxy
  deployment file we publish and those accounts are in every fixture,
  so without this rule they alone yield hundreds of findings. Computed
  from their definitions so the list cannot drift.
- Breadth downgrade. A value in three or more files is a shared
  constant, not a leak. Breadth never opens a finding, only lowers one.

Advisory to start: the workflow annotates but never blocks, so the real
false-positive rate can be observed before anyone's merge depends on it.
Runs on the diff only, so it costs nothing on private repos and can be
adopted elsewhere unchanged.

Two failure modes are closed deliberately. Liveness is the evidence the
tool runs on, so `--fail-on-verified` exits non-zero on an INCOMPLETE
scan as well as a confirmed hit: a gate that passes because the RPC was
rate-limited is worse than no gate. And the workflow carries no secret,
because it runs the scanner from the pull request's own checkout and
same-repository PRs do receive repository secrets — a PR editing scan.ts
could otherwise print a private RPC URL, API key included, to the log.

Keys are never printed in full, and nothing derived from an RPC URL
reaches the logs intact: an unnamed endpoint is displayed as its
hostname, and provider error text is sanitised too, since ethers formats
failures with the whole request URL and would otherwise put a provider
API key into a CI warning on any 401 or 429.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@maciejwitowski
maciejwitowski force-pushed the maciej/keyscan-private-key-detection branch from b8ff1e8 to d09e78c Compare August 11, 2026 09:45
maciejwitowski and others added 3 commits August 11, 2026 14:24
The scanner ran only in CI. This repository is public, so a CI check
reports an exposure that has already happened: once the push lands the
key is in every fork, clone and mirror, and GitHub keeps the object even
after a force-push. Detection after the fact is a smoke alarm that rings
once the house is gone.

Adds a pre-push hook running the same rules offline, in about two
seconds, over only the commits being pushed. `yarn install` points
core.hooksPath at .githooks, so it installs itself.

Verified against the real thing: replaying the commit that leaked the
deployer key blocks the push, names the right file and line, and derives
the right address, with no network.

Three changes were needed to get there.

Context is no longer line-scoped. A declaration and its value routinely
sit on different lines, which is exactly the shape that leaked:

    const FUNDER_PRIVATE_KEY = (process.env[...] ??
      "0x...") as Hex;

Matching only the key's own line saw a quoted string and nothing else,
so the name that makes it obviously a secret sat one line out of reach.
Candidates now carry two lines either side.

Periodic values are rejected. A key from a real random source is never a
short pattern repeated to fill 64 characters, so `1234567890abcdef` four
times over is a placeholder, not a secret. Another entropy statement
rather than a list. sha256("") joins the published constants for the
same reason keccak256("") already had.

A `keyscan: allow` marker retires a single finding. The only other
escape is --no-verify, which disables every hook for that push; given
just the blunt tool, people keep reaching for it and the check quietly
stops existing.

Measured offline on the tracked tree: unity-surfaces 8 findings -> 4,
vana-sdk 22 -> 11, personal-server-ts 4 -> 2, data-gateway 0 -> 0, and
vana-smart-contracts holds at 2, which are the two real keys.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three fixes from review.

The hook could fail open. For a new branch, if `git merge-base` returned
nothing — unrelated history, an orphan branch, an unresolvable default
ref — the loop skipped that ref and the push went through unscanned. Not
knowing what is in a push is not a reason to allow it, so it now refuses
and says how to proceed.

Range handling for force-pushes. `remote..local` compares the two end
states, so the added lines are exactly what the push introduces relative
to what the remote holds. The previous three-dot form diffed from the
merge base, which re-scanned commits that were already pushed and could
block on pre-existing content after a rebase.

postinstall no longer overwrites a developer's setting. It sets
core.hooksPath only when nothing is configured, and prints a line when
it does, rather than silently reassigning hooks during an install.

Verified: existing-branch push 0, no-merge-base 1, branch deletion 0,
new branch with a base 0, and the replayed leak commit still blocks.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The guard exited 0 when git rev-parse could not resolve the toplevel,
so a check that never ran reported success. A control that cannot run
must not pass. It now refuses and names --no-verify as the deliberate
escape.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@tnunamak

tnunamak commented Aug 11, 2026 •

Copy link
Copy Markdown
Member

Codex:

Thanks for adding a pre-push layer. Running before upload matters for public repositories, and the address and chain checks are useful as a second signal.

I reproduced four gaps that make this unsafe as the main enforcement path:

  1. git diff base..local checks the endpoint diff, not every commit being uploaded. If commit A adds a key and commit B removes it, both commits are pushed but the endpoint diff is clean.
  2. The zero-context diff cannot see an unchanged declaration near a new value. Adding a value below an existing privateKey declaration can produce no finding.
  3. The pre-push hook invokes the scanner with --offline, so chain validation is disabled at the point where it could prevent public exposure.
  4. CI is advisory: it uses continue-on-error and has no mode that fails on a verified result.

I would keep the semantic verifier as an advisory or scheduled second check. The shared scanner in vana-com/.github checks the complete pushed commit range in pre-push and CI. The draft integration for this repository is PR #70.

Making that CI check required still needs separate branch-protection approval because it changes merge behavior for everyone.

@tnunamak

Copy link
Copy Markdown
Member

Follow-up: I adapted the scalar validation, address derivation, retry, and Vana/Moksha liveness work from this PR into the shared workflow. The draft is vana-com/.github #1, and the source attribution to you and this PR is retained in the code and NOTICE.

The central design keeps Gitleaks as the authoritative detector and runs liveness as a second, advisory job. Both jobs inspect every introduced commit through the same materializer and run only code from the caller's immutable central-workflow pin. RPC requests contain derived addresses, not private-key candidates.

The public draft callers are live and both scanner jobs pass:

Your branch is unchanged. No branch-protection or ruleset settings were changed. Could you review the central draft? If the adapted version looks right, I suggest closing this PR in favor of the shared implementation after review.

@tnunamak

Copy link
Copy Markdown
Member

Codex: Maciej, thanks again for this work. We used the address-derivation and Moksha/mainnet verification approach from this PR in the shared implementation:

vana-com/.github#1

The smart-contracts rollout is here:

#70

The shared version adds full commit-range scanning and an explicit pre-push setup command, keeps the hook offline after setup, and avoids changing core.hooksPath during package installation. On-chain verification is advisory, while the offline lexical scan remains the blocking control.

I’m closing this as superseded because merging both implementations would create competing hooks and policies. Your work directly informed the verification layer.

@tnunamak tnunamak closed this Aug 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants