docs: document security threat model - #557
Conversation
lodekeeper-z
left a comment
There was a problem hiding this comment.
The structure and attacker/path/classification discipline are strong, but I found five threat-model accuracy gaps that should be resolved before this becomes the review baseline. The most important are the DA ordering claim and the process-wide pubkey-cache rollback/finalization description. Details are inline.
Review scope: AGENTS.md and THREAT_MODEL.md at b8d60d4. I also checked the corresponding Lodestar-ts block verification path and current Lodestar-z state-transition/cache code. CI is green on Ubuntu, ARM, bindings, title, and spec preparation; macOS was still pending at the time of review.
lodekeeper-z
left a comment
There was a problem hiding this comment.
Supplemental review after the independent security and consistency passes completed. These are distinct from the five findings in my first review and focus on boundary coverage and terminology.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6d54ee28ab
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
lodekeeper-z
left a comment
There was a problem hiding this comment.
Re-reviewed at 2639d32.
The split addresses my main maintainability concern: THREAT_MODEL.md is now a substantially shorter normative contract, while current integration status and implementation mechanics are isolated in docs/security/IMPLEMENTATION_MAP.md with ownership, a review date, and explicit update triggers in AGENTS.md.
The revised text also resolves the substantive issues from my earlier reviews: planned-path classification, host-process confidentiality, N-API validation wording, concurrent DA/STF gating, append-only pubkey-cache behavior, the JavaScript/native and Zig/native boundaries, cumulative exhaustion, state provenance versus eligibility, and the Gloas status distinction. The two deliberately out-of-scope cases are now stated consistently rather than left ambiguous.
I found no remaining blocking issues. Relative links and git diff --check pass. The primary build and binding checks are green; the remaining spec matrix is still running.
lodekeeper-z
left a comment
There was a problem hiding this comment.
Follow-up verification against the current Lodestar-ts and Lodestar-z sources found several accuracy gaps that the implementation map is intended to prevent. This review supersedes my earlier approval. The normative/implementation split remains a strong improvement; the requested changes are scoped corrections to the resulting baseline.
lodekeeper-z
left a comment
There was a problem hiding this comment.
Re-reviewed at 9848319. The follow-up resolves all five accuracy gaps from my changes-requested review: JavaScript and native Zig callers now have distinct capability models; authenticated anchors are included in eligible trusted pre-states; the BLS/pubkey-cache surfaces are correctly classified as exported but not wired into Lodestar-ts production; the current Lodestar-ts checkpoint decode/authentication path is documented; live-chain import is separated from archive-only backfill; and the process-global reused epoch-transition cache plus its borrowed-lifetime serialization requirements are explicit.
I cross-checked the changed wording against the current Lodestar-ts and Lodestar-z sources. git diff --check passes, all relative documentation links resolve, and the current CI checks are green or still running with no failures. No blocking issues remain.
lodekeeper-z
left a comment
There was a problem hiding this comment.
Re-reviewed the simplification at 552c360. The shorter runtime trust model preserves the earlier integration and shared-cache corrections, and the implementation-map links and source anchors validate. One scope regression remains: the rewrite drops supply-chain and release-artifact integrity from the protected assets, objectives, boundaries, and implementation map. That makes dependency, workflow, or artifact-substitution findings impossible to classify under the document's own required-objective framework. Please retain a concise supply-chain/release objective and boundary.
| - Ethereum consensus safety and liveness; | ||
| - state-transition, fork-choice, and BLS correctness; | ||
| - native memory safety and availability of the hosting process; and | ||
| - integrity of shared native state across calls and Node.js environments. |
There was a problem hiding this comment.
Blocking: The simplification removes source-dependency and native release-artifact integrity from the protected assets, and the later objectives/boundaries no longer cover build or release trust. Because classification now requires a violated security objective, a malicious dependency pin, workflow, or substituted published addon would fall outside this baseline even though it is a Lodestar-z security path. Please retain a concise artifact-integrity asset/objective and a build/release trust boundary; volatile workflow details can remain in the implementation map.
lodekeeper-z
left a comment
There was a problem hiding this comment.
Re-reviewed at 489787d. This restores the missing supply-chain and release-artifact integrity coverage across the protected assets, attacker model, security objectives, trust boundaries, and implementation map. I verified the cited dependency pins and release workflow controls (SHA-pinned actions, ReleaseSafe artifacts, frozen lockfile, and npm provenance); relative links and git diff --check pass. No blocking issues remain.
spiral-ladder
left a comment
There was a problem hiding this comment.
overall looks good, we can tweak the doc according as we find issues
🤖 I have created a release *beep* *boop* --- ## [1.0.0](v0.1.2...v1.0.0) (2026-08-19) ### Features * add `state.getBuildersLength()` binding ([#472](#472)) ([be2b5ab](be2b5ab)) * **beacon-node:** add block state cache and checkpoint datastore ([#452](#452)) ([2145faa](2145faa)) * bindings to `getExpectedWithdrawals` and native tweaks ([#350](#350)) ([f47bc66](f47bc66)) * **bindings:** add pubkey cache syncPubkeys ([#537](#537)) ([542779f](542779f)) * **bindings:** aggregate cached public keys by validator index ([#397](#397)) ([2f90603](2f90603)) * **bindings:** align `BeaconStateView` with `IBeaconStateView` ([#347](#347)) ([b8ec273](b8ec273)) * **bindings:** configurable pubkey cache growth step ([#481](#481)) ([133ef24](133ef24)) * **bindings:** expose more APIs for STF ([#444](#444)) ([7fe2609](7fe2609)) * **bls:** add small MSM for npoints < 32 ([#393](#393)) ([b430638](b430638)) * **blst:** use external buffers for blst operations ([#358](#358)) ([78e4678](78e4678)) * **ci:** conditionally publish bindings with tag ([#355](#355)) ([ea77919](ea77919)) * **clock:** add clock module for slot/epoch timing ([#354](#354)) ([385b077](385b077)) * **fork_choice:** add Prometheus metrics module ([#309](#309)) ([cbc9d8d](cbc9d8d)) * **forkchoice:** implement the forkchoice module ([#246](#246)) ([7c62a9b](7c62a9b)) * getSyncCommitteesWitness ([#367](#367)) ([ef77649](ef77649)) * implement `loadState` API and binding ([#165](#165)) ([f903519](f903519)), closes [#159](#159) * **metrics:** metrics bindings ([#455](#455)) ([dd41999](dd41999)) * migrate blst,pubkeys to use zapi js dsl ([#331](#331)) ([fcd26ca](fcd26ca)) * **pubkeys:** add getPubkeyBytes binding ([#555](#555)) ([4ca51cf](4ca51cf)) * publish ARM64 musl bindings ([#482](#482)) ([ac764c9](ac764c9)) * **shuffle:** add swap-or-not shuffling module and binding ([#559](#559)) ([c2db37c](c2db37c)) * split nextValue fn ([#464](#464)) ([b47faeb](b47faeb)) * support getLatestWeakSubjectivityCheckpointEpoch ([#366](#366)) ([dcf3883](dcf3883)) * update fulu deposit processing ([#442](#442)) ([064335c](064335c)) ### Bug Fixes * avoid set ([#484](#484)) ([2e25d97](2e25d97)) * better generation of rand scalar ([#388](#388)) ([74dce77](74dce77)) * **bindings:** accept `dontTransferCache` in processSlots for backward compatibility ([#460](#460)) ([65df5af](65df5af)) * **bindings:** check signature infinity by default ([#509](#509)) ([2f5f281](2f5f281)) * **bindings:** clean up failed async BLS work ([#527](#527)) ([1111b00](1111b00)) * **bindings:** free metrics writer on scrape failure ([#529](#529)) ([4c8d94a](4c8d94a)) * **bindings:** harden random aggregate scalars ([#528](#528)) ([8e89a63](8e89a63)) * **bindings:** log level for missing fields ([#435](#435)) ([08faf41](08faf41)) * **bindings:** misordering of print for cpu count ([#381](#381)) ([752a972](752a972)) * **bindings:** populate epoch participation for test fixtures ([#436](#436)) ([8dbdd2e](8dbdd2e)) * **bindings:** refcount Pool to fix teardown panic ([#352](#352)) ([23b2f68](23b2f68)) * **bindings:** roll back partial N-API initialization ([#491](#491)) ([31c5ebb](31c5ebb)) * **bindings:** size BLS thread pool by cgroup-aware CPU count ([#386](#386)) ([3ae9522](3ae9522)) * **bindings:** validate class types before unwrap ([#514](#514)) ([2fd2ad5](2fd2ad5)) * **bindings:** validate secret key hex length ([#517](#517)) ([136e415](136e415)) * **bls:** align PublicKey.uncompress validation with Signature.uncompress ([#508](#508)) ([5a8dbe9](5a8dbe9)) * **bls:** bound randomized aggregation inputs ([#548](#548)) ([779d0bf](779d0bf)), closes [#542](#542) * **bls:** clean up partial thread pool initialization ([#490](#490)) ([d55e598](d55e598)) * **bls:** convert pippenger scratch bytes to element counts ([#513](#513)) ([a12ca92](a12ca92)) * **bls:** enforce 32-byte signing roots ([#545](#545)) ([72fd308](72fd308)) * **bls:** make batch cardinality structural ([#547](#547)) ([a06d8b2](a06d8b2)) * **bls:** preserve aggregate outputs on failure ([#521](#521)) ([e0b6dd1](e0b6dd1)) * **bls:** reject empty keygen salts ([#524](#524)) ([d2a9c86](d2a9c86)) * **bls:** reject unknown BLST error codes ([#525](#525)) ([9e4a6ad](9e4a6ad)) * **bls:** size pairing buffers for 32-bit targets ([#531](#531)) ([dc64a27](dc64a27)) * **blst:** default signature infinity check to true if not provided ([#387](#387)) ([021cdcb](021cdcb)) * **build:** remove `zig-out` from `files` ([#360](#360)) ([c52af09](c52af09)) * **ci:** fix caching spec test version ([#439](#439)) ([96885a1](96885a1)) * dangling state pointer in loadOtherState ([#450](#450)) ([81cbd5f](81cbd5f)) * **epoch_cache:** compute missing `next_proposers` ([#447](#447)) ([0088a29](0088a29)) * **epoch_cache:** populate decision roots in afterProcessEpoch ([#453](#453)) ([4b70a5e](4b70a5e)) * export asyncAggregateWithRandomness through napi binding ([#371](#371)) ([1d04c2b](1d04c2b)) * harden memory safety across PMT, SSZ tree views, and state transition ([#377](#377)) ([d6f5897](d6f5897)) * improve atomic ordering in ThreadPool and NAPI init ([#310](#310)) ([4b0a1cc](4b0a1cc)) * interface compatbility with NativeBeaconStateView ([#445](#445)) ([89e13d1](89e13d1)) * missing deinits in loadOtherState ([#459](#459)) ([094d278](094d278)) * missing state commits ([#454](#454)) ([a432b55](a432b55)) * no-op when syncPubkeys run on a pk cache with shrinking validator set ([#432](#432)) ([ed05a99](ed05a99)) * param order in BeaconBlockBody ([#348](#348)) ([d8b9c06](d8b9c06)) * pendingConsolidations bindings ([#449](#449)) ([b9c497e](b9c497e)) * **pmt,ssz:** harden chunked-leaf and zero-copy tree-view memory safety ([#400](#400)) ([de50c53](de50c53)) * populate cache balances during rewards/penalties processing ([#474](#474)) ([5bf23dc](5bf23dc)) * re-expose sizes ([#369](#369)) ([64b81f3](64b81f3)) * remove `slashValidator` gating on active status ([#448](#448)) ([d319a0d](d319a0d)) * **ssz:** drop redundant default-init pass in fixed-list decode ([#468](#468)) ([0c757be](0c757be)) * **ssz:** publish child cache entries after lookup ([#565](#565)) ([21e78c9](21e78c9)) * state transition binding exports ([#456](#456)) ([895982c](895982c)) * **state-transition:** group-check signature sets ([#515](#515)) ([42774e9](42774e9)), closes [#502](#502) * **state-transition:** isolate epoch step cache mutations ([#535](#535)) ([a83741a](a83741a)) * **state-transition:** repair Pool.init call broken by [#346](https://github.com/ChainSafe/lodestar-z/issues/346)×[#367](https://github.com/ChainSafe/lodestar-z/issues/367) merge skew ([#394](#394)) ([b42944f](b42944f)) * various fixes around config ([#433](#433)) ([c4f082c](c4f082c)) ### Performance Improvements * **bindings:** drop TS BLS comparison benches and report benchmarks on PRs ([#552](#552)) ([c909c6f](c909c6f)) * **bls:** add cache-aware signature verifier ([#562](#562)) ([063857e](063857e)) * **bls:** bypass worker queue for small batches ([#553](#553)) ([3f8a6df](3f8a6df)) * **epoch:** replace AutoHashMap with array lookup in reward/penalty caches ([#286](#286)) ([e4e181b](e4e181b)), closes [#243](#243) * **pmt:** chunked-leaf packing for basic lists and container_struct ([#346](#346)) ([ba156c4](ba156c4)) ### Code Refactoring * allocate `AsyncAggRandData` in one obj ([#384](#384)) ([459750f](459750f)) * **bindings/pubkeys:** simplify allocation strategy for aggregate ([#518](#518)) ([b82750f](b82750f)) * **bindings:** rename blst Lifecycle to State ([#516](#516)) ([0a9c179](0a9c179)) * **bindings:** use zapi js.io() instead of local io module ([#469](#469)) ([2b34cc0](2b34cc0)) * **bindings:** wake only required number of workers ([#383](#383)) ([1db57f1](1db57f1)) * **bls:** allocations around VMAS ([#395](#395)) ([dfda58c](dfda58c)) * **bls:** clean up bls ([#398](#398)) ([e0f3b9b](e0f3b9b)) * **bls:** remove need for tracking results for verifyMultipleAggregateSignatures ([#389](#389)) ([6fe5c3f](6fe5c3f)) * **bls:** remove single-threaded fallback ([#390](#390)) ([e057713](e057713)) * **clock:** single public Clock; internalize SlotClock ([#463](#463)) ([fbab1fa](fbab1fa)) * make XXXDecisionRoot fns return `js.String` ([#342](#342)) ([aef4420](aef4420)) * move shuffle into swap_or_not_shuffle module ([#558](#558)) ([e56efb2](e56efb2)) * **pubkeys:** centralize the process-wide cache ([#522](#522)) ([dc9669d](dc9669d)) ### Miscellaneous Chores * avoid slow tests in AGENTS.md ([#546](#546)) ([c60f2a9](c60f2a9)) * bump zapi to include musl build ([#485](#485)) ([0b488cc](0b488cc)) * **ci:** pin github actions with sha hashes ([#507](#507)) ([167b8f5](167b8f5)) * deprecate unused blst APIs ([#575](#575)) ([7b547fa](7b547fa)) * **deps:** bump zapi v2.1.0 -> v2.2.0 ([#376](#376)) ([0c240d8](0c240d8)) * **deps:** bump zbuild ([#403](#403)) ([e2545de](e2545de)) * **deps:** compile blst with ReleaseFast ([#391](#391)) ([753a896](753a896)) * **deps:** update zapi to 3.1.0 ([#483](#483)) ([f3e5827](f3e5827)) * **deps:** use zapi v2.1.0 ([#372](#372)) ([88f403a](88f403a)) * disable gemini auto code review ([#382](#382)) ([63e42a4](63e42a4)), closes [#380](#380) * **docs:** add comments section in AGENTS.md ([#566](#566)) ([0c09750](0c09750)) * move state clones out of benchmark run functions ([#324](#324)) ([e4035de](e4035de)) * prepare 1.0.0 release ([#576](#576)) ([20b657b](20b657b)) * release v0.1.2-rc.3 ([#370](#370)) ([e4fc551](e4fc551)) * **release:** 0.1.2-rc.2 ([#365](#365)) ([7046128](7046128)) * **release:** v0.1.2-rc.10 ([#477](#477)) ([9a4fad5](9a4fad5)) * **release:** v0.1.2-rc.4 ([#373](#373)) ([09468f1](09468f1)) * **release:** v0.1.2-rc.5 ([#374](#374)) ([f344efa](f344efa)) * **release:** v0.1.2-rc.6 ([#375](#375)) ([bdf5b67](bdf5b67)) * **release:** v0.1.2-rc.8 ([#401](#401)) ([06f91c2](06f91c2)) * **release:** v0.1.2-rc.9 ([#404](#404)) ([6024800](6024800)) * remove merge transition code ([#359](#359)) ([09b175d](09b175d)) * remove stale epoch cache TODOs ([#534](#534)) ([27a547a](27a547a)) * rename era shortHistoricalRoot to shortEraRoot ([#473](#473)) ([c75a4d3](c75a4d3)) * **scripts:** build bindings with preset ([#434](#434)) ([a1b5ef7](a1b5ef7)) * silence debug log when used in release builds ([#486](#486)) ([c5377d7](c5377d7)) * support dev workflow ([#364](#364)) ([fcb9a78](fcb9a78)) * update gloas types to align with the latest specs ([#431](#431)) ([1f065b5](1f065b5)) * update spec test version to v1.7.0-alpha.11 ([#451](#451)) ([5875660](5875660)) * update spec-test-version: v1.6.0-beta.2 -> v1.7.0-alpha.10 ([#441](#441)) ([f932b1c](f932b1c)) * update zapi to 4.0.0 ([#571](#571)) ([de8e3fd](de8e3fd)) ### Documentation * document security threat model ([#557](#557)) ([e678b87](e678b87)) * more comprehensive AGENTS.md ([#520](#520)) ([c74b386](c74b386)) * **pkix:** document load provenance requirement ([#556](#556)) ([37e0aa2](37e0aa2)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Summary
AGENTS.mdto consult both documents and revisit them when security-relevant integration boundaries changeWhy
Security reports have produced faulty findings because Lodestar and Ethereum trust assumptions were implicit. In particular, agents treated trusted beacon states and local ERA/database data as hostile, confused ancestry checks with full validation, and overlooked validation performed by Lodestar-ts.
Separating the stable security contract from current implementation mappings also reduces verbosity and limits documentation bitrot. The normative core changes only when trust or classification changes; implementation details can evolve in the audit map.
Impact
Documentation only. Maintainers and automated reviewers gain a shorter basis for distinguishing security vulnerabilities, integration-readiness issues, consensus bugs, boundary hardening, trusted-input reliability problems, and operator misuse.
AI assistance
This documentation was primarily AI-authored through an extended collaborative threat-modeling discussion with maintainer-provided Lodestar and Ethereum domain guidance.