fix(keystore): the compiled-in dev trust anchor was not a point on the curve

[Kartikey1306]

Merge-order constraint — do not merge this PR without #122. This change makes the compiled-in dev anchor a real curve point whose private half is printed in RFC 8032 §7.1, and every board release.yml publishes falls back to that anchor (no board under boards/ implements otp_read). At this head nothing wires EBLDR_PRODUCTION_KEY, so a master that has this PR without #122 would let anyone sign firmware a released eBoot accepts. The fix here is correct and must not be reverted — the exposure is about sequencing. Land #115 → #116 → #122 in one batch, or land #122's configure-time FATAL_ERROR guard first, and do not tag a release from a master that has this without #122.

Stacked on #115 (master does not build without it); the diff of this PR alone is core/keystore.c (+6/−2 in the key array plus a comment), tests/unit/test_keystore.c (+53) and one corrected comment in tests/unit/test_secure_boot_policy.c.

Problem

core/keystore.c's default_dev_key says — in a #warning that fires on every dev build, in the comment above it, and in a note I wrote in #82 — that it is the RFC 8032 §7.1 TEST 1 public key. That claim is the only thing that makes a compiled-in anchor usable for development: the RFC prints the matching secret, so anyone can sign a test image for a board that falls back to it.

It was not that key. Verified:

RFC 8032 TEST 1 public:  d75a980182b10ab7d54bfed3c964073a0ee172f3daa62325af021a68f707511a
core/keystore.c (v0.1.0→): d75a980182b10ab7d54bfed3c964073a0ee172f3daa3f4a18c42c47684377725
                                                       ^^ diverges at byte 21

and the shipped bytes do not decode to a point on edwards25519 (decode y, recover x², check x² against the curve equation: no root). So unpackneg() rejects it before any arithmetic, and eos_ed25519_verify() returns EOS_ERR_SIGNATURE for every signature ever presented to a board on the compiled-in path.

That has been true since v0.1.0 and mattered little while nothing verified unconditionally. Since #104 (eos_fw_update_finalize() verifies the signature always, before anti-rollback), firmware update refuses every image on every board without OTP. It is also why #115 provisions its signing key through simulated OTP rather than rely on the fallback.

Fix

The array is now the RFC's 32 bytes. Nothing else in the fallback path changes.

Test

test_keystore.c::test_compiled_in_anchor_verifies_its_own_rfc_vector detaches the OTP (so the keystore takes the compiled-in path), takes the active key, and:

1. asserts the bytes are the RFC's;
2. verifies the RFC's own TEST 1 signature over the empty message with it — the load-bearing check;
3. shows the accept discriminates: the same signature over "x" is refused, and a bit-flipped signature over the empty message is refused.

Verified the test fails without the fix: with the old bytes, step 2 returns EOS_ERR_SIGNATURE (−4). With the fix: 10/10 keystore tests, 31/31 suite.

Not changed

• EBLDR_PRODUCTION_KEY and the #warning behave as before; a production build never sees this array.
• The keystore's OTP-first policy (fix(keystore): fail closed when the OTP trust anchor cannot be read #59) is untouched — this is only the value a no-OTP board falls back to.

Review follow-ups (2026-09-14)

Stacked on #115; review e152d8e..ef5b70d for this change alone (corrected from a276016..31f44fb at 12:10 IST 2026-09-15: #115 was rebased onto master 682d005 as e152d8e, and this branch was restacked onto it — two commits replayed clean, verified on the new head before the push: build OK, ctest 31/31, pytest 55 passed / 3 skipped) — two commits (the rebased fix, and the follow-up), three files: core/keystore.c, tests/unit/test_keystore.c, CHANGELOG.md, plus a corrected comment in tests/unit/test_secure_boot_policy.c.

• CHANGELOG (approver's ask 1): Security bullet stating the behaviour change plainly — a board with no OTP and no EBLDR_PRODUCTION_KEY goes from refusing every image (since fix(fw-update): verify the image signature unconditionally at install #104) to accepting images signed with the public RFC test key, which is the documented development intent and a key that must never reach a device.
• Third copy of the TEST 1 public key (approver's optional ask): test_keystore.c now takes it from tests/vectors/fw_update_test_sigs.h (eos_test_sig_pubkey, derived from the RFC secret by tools/gen_fw_update_test_sigs.py and pinned to that script by tests/unit/test_fw_update_test_sigs.py), so the compiled-in anchor is compared against an independently derived copy rather than a third literal.
• Finding 1 (P0, the release path reaches the dev key) is build: a Release build of a real board refuses to configure without a production trust anchor #122, stacked on this PR. Finding 2 (the production branch does not build) is fixed there too: ebldr_production_key[] is generated from EBLDR_PRODUCTION_KEY and the branch is compiled and run on every host build.

Verified on the current head: ctest 31/31, eboot_test_keystore 10/10, pytest 79 passed; with the old bytes restored the anchor test fails.

Closing issue

Fixes #119

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 93.33333% with 2 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
tests/unit/test_fw_update_test_sigs.py	93.33%	1 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

[srpatcha]

Review — eBoot#116 "fix(keystore): the compiled-in dev trust anchor was not a point on the curve"

head: f0aa1ba author: Kartikey1306 ci: pass (25 green, 0 red; only Create GitHub Release skipped)

Verdict: The diagnosis is exactly right and I confirmed it independently, off the
repo's own code: the old 32 bytes are not a point on edwards25519, the new bytes are
the RFC 8032 §7.1 TEST 1 public key, and the first divergence is at byte index 21 — the
PR says "agreed for 21 bytes and then did not", and that is literally true. The
regression test is real: reverting the bytes makes it fail. Land it.

One thing has to be said alongside it, and it is not a reason to hold the PR. This fix
takes a trust anchor that was inert and makes it work — and its private half is
printed in a public IETF document. Nothing structural stops that anchor being compiled
into the firmware release.yml publishes, and today it is.

Findings

#	Severity	File:line	Finding	Recommended fix
1	Critical (P0)	.github/workflows/release.yml:51,80,110,155,198; core/keystore.c:28-29	The release path reaches the development key, and after this PR that key works. EBLDR_PRODUCTION_KEY appears nowhere except inside core/keystore.c — there is no CMake option for it and no build sets it. All six firmware jobs in release.yml configure with only -DEBLDR_BOARD=… -DCMAKE_BUILD_TYPE=Release, so every published artifact compiles the #ifndef EBLDR_PRODUCTION_KEY branch. On any board where eos_keystore_init() finds no OTP, slot 0 is filled from default_dev_key and ks->source = EOS_KEY_SOURCE_COMPILED. That anchor is RFC 8032 §7.1 TEST 1, whose secret scalar is printed in the RFC, so anyone can sign firmware such a board will accept as authentic. The sole guard is a compile-time #warning, which does not fail a build and is invisible in a release log. .ai/security.md is explicit: "Test keys and development keys must be structurally incapable of signing a release artifact. Check that the release path cannot reach them." It reaches them. Pre-existing — this PR did not create it — but until now the anchor could not verify anything, so the exposure was latent; this correct fix makes it live.	Make it structural, not advisory. Add a real EBLDR_PRODUCTION_KEY CMake option and fail the configure when a release-shaped build does not set it: if(CMAKE_BUILD_TYPE STREQUAL "Release" AND NOT EBLDR_BOARD STREQUAL "none" AND NOT EBLDR_PRODUCTION_KEY) message(FATAL_ERROR ...). Then set it in the five release.yml firmware jobs. Smallest interim step if that is too much for one PR: have release.yml pass -DEBLDR_PRODUCTION_KEY=… and add a job that greps the built binary for the dev anchor bytes and fails on a hit. Track as P0 regardless of where it lands.
2	Medium	core/keystore.c:122-127	The production branch does not build. When EBLDR_PRODUCTION_KEY is defined, the code does extern const uint8_t ebldr_production_key[EOS_ED25519_PUB_KEY_SIZE]; — and nothing in the tree defines that symbol, nothing generates it, and no documentation says where an integrator is meant to put it. Following the #warning's own instruction therefore produces an undefined-reference at link with no guidance. Under §28's status policy this is a planned capability that reads as implemented: the comment at :25 states "Production builds set EBLDR_PRODUCTION_KEY, which replaces it" as settled fact.	Either provide the missing piece — a keys/production_key.c.in plus a CMake path that generates it from a PEM or raw file — or downgrade the comment to say plainly that the production anchor is not yet wired and name the issue tracking it. Do not leave a security-critical branch that cannot compile described in the present tense.
3	Low	(whole PR)	This PR is stacked on #115, not independent: 23 files change against master, but only three against 4d8696c (core/keystore.c, tests/unit/test_keystore.c, a comment in test_secure_boot_policy.c). Its 25 green checks therefore certify #115's work as much as its own. That is fine and the stacking is sensible — the test needs a tree that compiles — but a reviewer reading the file list cold will over-estimate the change, and if #115 is revised this PR must be rebased rather than merged as-is.	Add one line to the PR body: "Stacked on #115; review 4d8696c..f0aa1ba for this change alone." Merge #115 first.

Verification performed for this review

Detached scratch worktree under .ai/autoreview/state/verify/; the user's checkout was
untouched and nothing was pushed.

Check	Result
Is the old array a valid edwards25519 point? Solved x² = (y²−1)/(dy²+1) mod 2²⁵⁵−19 independently in Python	No — no square root exists. Confirms "not a point on the curve".
Is the new array a valid point?	Yes
Does the new array equal RFC 8032 §7.1 TEST 1 d75a9801…f707511a?	Yes, byte-for-byte
First index where the old array diverges from the RFC	21 — matches the PR's claim exactly
cmake -B build/host -DEBLDR_BUILD_TESTS=ON → cmake --build	PASS, no errors
ctest --test-dir build/host --output-on-failure --no-tests=error	PASS — 100%, 31/31
test_keystore verbose	PASS — 10/10, including test_compiled_in_anchor_verifies_its_own_rfc_vector
Negative control: reverted core/keystore.c to the pre-fix bytes, rebuilt, re-ran	test_compiled_in_anchor_verifies_its_own_rfc_vector [FAIL] at test_keystore.c:305, suite 0/1. The test genuinely pins the fix rather than passing vacuously.

I ran the negative control because a test added in the same commit as the fix it guards is
worth nothing until someone shows it can fail. This one can.

On the test itself

test_compiled_in_anchor_verifies_its_own_rfc_vector is well built for what it has to
prove. It calls otp_detach() first, so it exercises the compiled-in path specifically
rather than a provisioned one; it asserts the bytes match the RFC and that they verify
the RFC's own signature, so a future edit that swaps in some other valid curve point still
fails; and the two negative checks — same signature against a different message, and a
bit-flipped signature against the empty one — rule out a verifier that returns EOS_OK
unconditionally, which is the failure mode that would make the positive assertion
meaningless. That last part matters here more than usual, because eos_ed25519_verify()
is itself code this repo has had to repair twice (TASKS.md:56 records a version that
rejected every valid signature).

Also worth recording: test_failed_otp_read_does_not_fall_back_to_the_compiled_key
already existed and still passes. The fallback is gated on !otp_present, not on "OTP
read failed", so a board with OTP that cannot be read does not silently drop to the
dev anchor. That is the correct fail-closed shape and it is unaffected by this change.

Architecture conformance

Conforms. §21: eBoot is Tier 1 Foundation; core/keystore.c and tests/unit/ are both in
the owning repo. No #include, link line or manifest dependency changes, so §5.1's
dependency direction is untouched and no edge points up a tier. §14.1 ("Integrate key
management across eBoot, eSec, eOTA and release signing") is served by the fix and, as
finding 1 describes, is where the surrounding gap lives — the design treats key management
as one integrated system but says nothing about a release build being required to carry a
production anchor. I have appended a proposal for §14.1 to
.ai/autoreview/proposals/2026-09.md. §5.1's "eBoot keeps the trusted computing base
minimal and auditable" is the clause finding 1 engages: an anchor nobody could verify
against was not auditable, and an anchor anybody can sign against is not a boundary.

Proposed changes

Merge order:
  1. #115  (base of this stack)
  2. #116  (this PR) — rebase if #115 changes

Follow-up, P0, separate PR:
  CMakeLists.txt   add option(EBLDR_PRODUCTION_KEY ...) and a FATAL_ERROR guard
                   for Release + real board builds
  release.yml      pass -DEBLDR_PRODUCTION_KEY to all six firmware jobs
  core/keystore.c  supply or generate `ebldr_production_key`, or mark the branch
                   explicitly unimplemented per §28

Not checked

• I did not confirm that any shipped board actually lacks OTP. Finding 1's impact is
  bounded by how many EBLDR_BOARD targets reach the !otp_present fallback at runtime.
  I read the gate at core/keystore.c:121 but did not audit the six release boards'
  otp_read implementations, so the real blast radius is unknown — it could be all
  six or none. That is the first thing to establish before rating the follow-up work.
• No released artifact was inspected. I did not download an eBoot release and grep it
  for the anchor bytes; finding 1 is derived from reading release.yml and keystore.c,
  not from a binary.
• Cross-compile and fuzz jobs were not run locally. CI reports them green on this head
  and I did not read the logs.
• pytest tests/ — NOT RUN, pytest is not installed on this host.
• I verified the new key against RFC 8032 by computation and by the repo's own verifier,
  but I did not fetch RFC 8032 to re-read §7.1; the vector is reproduced from the test file
  and cross-checked for curve membership, which is the property that actually matters.
• Whether tools/sign_image.py and tools/eos_sign.py default to this same key, and so
  whether a developer signing locally would notice the change, was not examined.

---

Automated architecture review of f0aa1ba3a462 — scheduled, model claude-opus-5, checked against the EmbeddedOS Master Design v2.0. Advisory only: this reviewer never approves, requests changes, or merges. Reply here to discuss or push back — a wrong finding is a bug worth reporting.

[srpatcha]

Verified independently and it holds: decoding the old default_dev_key as an edwards25519 point fails (no square root exists for its y), while the new bytes decode, match RFC 8032 section 7.1 TEST 1, and are exactly what deriving from the RFC's secret produces. First divergent byte is 21, as you said. On your branch test_keystore is 10/10 and the full suite 31/31; with the old bytes restored the new test fails at tests/unit/test_keystore.c:305, so it's a genuine guard. Nice catch — a trust anchor that nothing can verify against is the kind of thing that hides for years.

Two small asks:

1. CHANGELOG.md — please add a line under Unreleased. Behaviourally, boards with no OTP and no EBLDR_PRODUCTION_KEY go from "refuse every image" (post-#104) to "accept images signed with the public RFC test key". That is the documented dev intent per the #warning at core/keystore.c:29, but it should be stated where people look before shipping.
2. 2. Stacking: this branch carries #115's commits, so the diff shows 23 files. Please note the dependency on #115 (or rebase once it lands) so the 3-file change is what reviewers see.
      Optional: tests/unit/test_keystore.c:292-299 is the third copy of the TEST 1 public key in the tree (also keystore.c:35 and tests/vectors/fw_update_test_sigs.h:14). Approving; merge after #115.

[Kartikey1306]

Thanks. Both asks done, and the P0 is now its own PR:

• CHANGELOG Security bullet states the change: a board with no OTP and no EBLDR_PRODUCTION_KEY goes from refusing every image (since fix(fw-update): verify the image signature unconditionally at install #104) to accepting images signed with the public RFC test key — the documented development intent, and a key that must never reach a device.
• Stacking: rebased on fix: repair master after the 09-07 batch merge #115's current tip; this change alone is a276016..31f44fb (corrected: the range first quoted here, 976ab0c..154c8e3, was pre-rebase and no longer resolves; the PR body had the right one) (core/keystore.c, tests/unit/test_keystore.c, CHANGELOG.md, one corrected comment). The PR body says so now.
• Third copy of the key: test_keystore.c now compares the anchor against eos_test_sig_pubkey from tests/vectors/fw_update_test_sigs.h — derived from the RFC secret by the generator and pinned to it by test_fw_update_test_sigs.py — instead of carrying its own literal.
• Finding 1 (P0) and finding 2 (the production branch does not build): build: a Release build of a real board refuses to configure without a production trust anchor #122, stacked on this PR. It adds the EBLDR_PRODUCTION_KEY option with a generated ebldr_production_key[], a configure-time refusal for a Release board build without a key (EBLDR_ALLOW_DEV_KEY=ON is the explicit opt-out), the secret on all 8 release configures, an artifact scan for the dev bytes, and a C suite that compiles the production branch on every host build. On your "Not checked" item: no board under boards/ implements otp_read, so the blast radius was all eight release boards — measured, in build: a Release build of a real board refuses to configure without a production trust anchor #122's body.

Current head: ctest 31/31, eboot_test_keystore 10/10, pytest 80 passed (corrected from 79 — see the note on #115); old bytes restored → the anchor test fails at the memcmp.

[srpatcha]

Review — eBoot#116 "fix(keystore): the compiled-in dev trust anchor was not a point on the curve"

head: 31f44fb author: Kartikey1306 ci: pass (26 green, 0 red; only Create GitHub Release skipped)

Verdict: Follow-up review. The two review-asks on the PR itself are done and I verified
both. The P0 was correctly scoped out to #122 rather than fixed here — which is the right
call for reviewability, but it means this PR, merged alone, is the commit that arms the
exposure. That is now the only thing I have to say about it, and it is a merge-order
constraint, not a defect in the diff.

Status of the previous findings (eBoot-116-f0aa1ba3.md)

#	Prev. severity	Status	Evidence
1	Critical (P0) — release path reaches the dev key	Open. Moved, not fixed.	Deliberately deferred to #122 (fix/production-trust-anchor-is-structural, open, reviewed in this same run) and tracked as issue #120. Nothing in this head changes it: grep -rn EBLDR_PRODUCTION_KEY CMakeLists.txt .github/ returns nothing, so every release.yml firmware job still configures with only -DEBLDR_BOARD=… -DCMAKE_BUILD_TYPE=Release and still compiles the #ifndef branch. See finding 1 below — the rating goes up, not down, at this head.
2	Medium — the production branch does not build	Open. Moved, not fixed.	Unchanged at this head: core/keystore.c:125 still declares extern const uint8_t ebldr_production_key[…] and the only two hits for that symbol in the whole tree are that declaration and the memcpy on the next line. Nothing defines it, nothing generates it. Fixed in #122.
3	Low — stacking not stated in the PR body	Resolved.	The body now carries "Stacked on #115; review a276016..31f44fb for this change alone" and enumerates the three files. I checked the range: it is correct, and it is the two commits 7370d73 + 31f44fb. (The PR comment quotes 976ab0c..154c8e3, which is a pre-rebase range and no longer resolves; the body is the one that is right.)

Also addressed, from the previous review's prose rather than its table: the third
hand-typed copy of the key is gone. tests/unit/test_keystore.c:309 now compares the anchor
against eos_test_sig_pubkey from the generated fixture header — a copy derived from the RFC
secret by tools/gen_fw_update_test_sigs.py and pinned to that script by
tests/unit/test_fw_update_test_sigs.py — instead of a literal. The negative control below
fails at exactly that memcmp, so the cross-check is real.

And the previous review's first "Not checked" item is now answered. It asked how many
boards actually reach the !otp_present fallback, and said the blast radius was unknown. It
is every board: no file under boards/ assigns .otp_read at all — grep -rn otp_read boards/ returns zero hits across all 82 board directories, and the field is not even set to
NULL explicitly, it is zero-initialised. hal/hal_core.c:160 returns
EOS_ERR_NOT_SUPPORTED when the pointer is null, core/keystore.c:71 reads that as "no OTP",
and :114 takes the compiled-in branch. So all eight boards built by release.yml (the
matrix job plus rpi4, riscv64_virt, esp32, esp32c3, x86_64_efi) use the compiled-in
anchor. The author's measurement matches mine.

Findings

#	Severity	File:line	Finding	Recommended fix
1	Critical (P0)	core/keystore.c:114-127; .github/workflows/release.yml:51,80,110,155,198,227	Carried forward, and it is this PR that makes it live. Before this change the dev anchor was off-curve, so it verified nothing: the exposure was latent and the practical effect was a denial of service (since #104, firmware update refused every image on every board). This PR makes the anchor a real edwards25519 point whose private half is printed in RFC 8032 §7.1, and — per the board audit above — every one of the eight boards release.yml publishes falls back to it. The moment this lands without #122, anyone who can read a public IETF document can sign firmware that a released eBoot accepts as authentic. .ai/security.md: "Test keys and development keys must be structurally incapable of signing a release artifact. Check that the release path cannot reach them." It reaches them, and the sole guard is a #warning that does not fail a build. The fix in this PR is correct and should not be reverted — leaving a broken anchor in place is not a security control. The severity is about sequencing.	Do not merge #116 into master without #122 in the same batch. #122 is stacked on this PR and adds the EBLDR_PRODUCTION_KEY option, the configure-time FATAL_ERROR for a Release board build without one, the key on all eight release configures, and an artifact scan for the dev bytes. Merging #115 → #116 → #122 in order leaves a window on master where the exposure is armed; if anything can tag a release from master in that window, close the window by merging #116 and #122 together, or by landing #122's CMake guard first. If #122 is going to sit in review, hold #116 with it.
2	Medium	core/keystore.c:125-126	Carried forward. The EBLDR_PRODUCTION_KEY branch still cannot link: extern const uint8_t ebldr_production_key[…] has no definition anywhere in the tree, so an integrator who follows the #warning's own instruction gets an undefined reference and no guidance. Per §28's status policy this is a planned capability written in the present tense — the comment at :25 states the replacement as settled fact. Not a regression from this PR and correctly deferred; recorded so it is not lost if #122 changes shape.	Resolved by #122 as written (it generates ebldr_production_key[] and compiles the branch on every host build). Verified there, not here. If #122 is reworked, this must survive the rework.

No new defects in the two commits at this head. I looked: the diff is the 16 key bytes, a
comment, one 53-line test, one CHANGELOG bullet, and a correction to a stale note in
test_secure_boot_policy.c that had asserted the shipped key was the RFC key. That last
one is the author correcting their own earlier comment, which is the right instinct.

Verification performed for this review

Detached worktree at .ai/autoreview/state/scratch/eBoot-116 off refs/pull/116/head. The
user's eBoot checkout was not touched and is still clean; nothing was committed or pushed.

Check	Result
cmake -B build/host -DEBLDR_BUILD_TESTS=ON -DCMAKE_BUILD_TYPE=Debug → cmake --build --parallel 4	PASS, no errors
ctest --test-dir build/host --output-on-failure --no-tests=error	PASS — 31/31, 11.06s
eboot_test_keystore verbose	PASS — 10/10, including test_compiled_in_anchor_verifies_its_own_rfc_vector
Negative control: restored the pre-fix 16 bytes in core/keystore.c, rebuilt the suite, re-ran	test_compiled_in_anchor_verifies_its_own_rfc_vector [FAIL] at test_keystore.c:309, memcmp(key, eos_test_sig_pubkey, 32) == 0 — 9/10. The test pins the fix, and it now fails against the generated fixture key rather than a hand-typed literal, which is precisely what the previous review asked for. core/keystore.c was restored; git diff --stat clean afterwards.
Blast-radius claim: "no board under boards/ implements otp_read"	CONFIRMED — zero hits across 82 board directories; the .otp_read member is never assigned, so it is null and eos_hal_otp_read() returns EOS_ERR_NOT_SUPPORTED on every target.
Is EBLDR_PRODUCTION_KEY wired anywhere at this head?	NO — no hits in CMakeLists.txt or .github/. Finding 1 stands unmitigated at this head.
Is ebldr_production_key defined anywhere at this head?	NO — two hits total, both the declaration and its use. Finding 2 stands.
CHANGELOG's "#120 tracks making that structural"	CONFIRMED — #120 is an open issue, "P0: release builds can reach the development trust anchor, and nothing structural stops them". #122 is the PR. Both references are right; they point at different things.
PR body's stacking range a276016..31f44fb	CONFIRMED — two commits, and the file set matches.

Architecture conformance

Conforms. §21: eBoot is Tier 1 Foundation; core/keystore.c and tests/unit/ are in the
owning repo. No #include, link line or manifest dependency changes at this head, so §5.1's
dependency direction is untouched and nothing points up a tier. §14.1 ("Integrate key
management across eBoot, eSec, eOTA and release signing") is the clause finding 1 lives under,
and the previous review already appended a §14.1 proposal to proposals/2026-09.md asking the
master design to require that a release build carry a production anchor — that proposal stands
and I have not duplicated it. §5.1's "eBoot keeps the trusted computing base minimal and
auditable" is served by the fix itself: an anchor that decodes to nothing is not auditable,
and the new test is the first thing in the repo that ever asked the fallback key to verify
anything.

Proposed changes

Merge order — the sequencing IS the mitigation:
  1. #115
  2. #116 (this PR)  ]  land together, or land #122's CMake guard first.
  3. #122            ]  Do not tag a release from a master that has 2 without 3.

Nothing to change in this diff.

Not checked

• No released artifact was inspected. Finding 1 is derived from reading release.yml,
  core/keystore.c, hal/hal_core.c and every boards/*/board_*.c — not from downloading a
  published .efw and grepping it for d75a9801…. The inference is strong but it is an
  inference.
• Cross-compile (STM32F4, Cortex-M4), fuzz-harness build, CodeQL, cppcheck/clang-tidy —
  NOT RUN locally. CI green on this head; logs not read.
• pytest tests/ — NOT RUN at this head. I ran it at #115's head (80 passed) and the two
  commits here touch no Python, so I expect it unchanged — but I did not execute it here and
  the PR comment's "79 passed" is unverified at this SHA.
• ASan/UBSan and Valgrind — NOT RUN.
• Whether tools/sign_image.py / tools/eos_sign.py default to this same key — still not
  examined, carried over from the previous review. It matters for whether a developer signing
  locally would notice the change.
• RFC 8032 was not re-fetched. The previous review established curve membership and the
  byte-21 divergence by computation; I did not redo that work, and relied on it plus the
  passing/failing verifier.

---

Automated architecture review of 31f44fb835a0 — scheduled, model claude-opus-5, checked against the EmbeddedOS Master Design v2.0. Advisory only: this reviewer never approves, requests changes, or merges. Reply here to discuss or push back — a wrong finding is a bug worth reporting.

[Kartikey1306]

Agreed in full, and the merge-order constraint is now the first thing in the PR body, where a merger sees it before the diff: do not merge #116 without #122, land #115 → #116 → #122 as one batch or land #122's configure-time guard first, and do not tag a release from a master that has this without #122. Your board audit and mine agree — no board under boards/ assigns .otp_read, so every one of the eight release boards falls back to the compiled-in anchor, and at this head nothing wires EBLDR_PRODUCTION_KEY. The fix here stays; the severity is about sequencing, exactly as you put it.

Two stale numbers of mine, corrected in the earlier comment with the correction left visible: the stacking range quoted there was the pre-rebase 976ab0c..154c8e3, which no longer resolves — the body's a276016..31f44fb is the right one (two commits: 7370d73, 31f44fb); and pytest 79 → 80, re-run at a276016 with EOS_REQUIRE_SIGNING_TESTS=1.

Nothing pushed to this branch — nothing in the diff to change, and a push here would re-stack #122.

[srpatcha]

Review — eBoot#116 "fix(keystore): the compiled-in dev trust anchor was not a point on the curve"

head: ef5b70d author: Kartikey1306 ci: pass (26 green, 0 red; only Create GitHub Release skipped)

Verdict: Third look. This head is a pure re-stack onto #115's new tip — the two
commits that are this PR's own work are byte-identical to the reviewed 31f44fb8 by
git patch-id, and nothing else changed. Both carried findings therefore stand exactly as
written, re-verified against this head rather than recalled. The fix itself remains correct
and should land. One new finding: the only documented way for a developer to embed their
own trust anchor writes a header that nothing in the tree includes, under a symbol the code
deliberately stopped using — and that survives #122 as well.

Status of the previous findings (eBoot-116-31f44fb8.md)

#	Prev. severity	Status	Evidence at this head
1	Critical (P0) — the release path reaches the development key, and this PR makes that key work	Untouched, and correctly so. Still open; it is a sequencing constraint, not a diff defect.	Re-checked, not recalled: grep -rn EBLDR_PRODUCTION_KEY CMakeLists.txt .github/ returns nothing at ef5b70d0, so every release.yml firmware job still compiles the #ifndef branch. grep -rln otp_read boards/ returns 0 of 82 board directories, so every release board falls back to the compiled-in anchor. The build log confirms the sole guard is advisory: core/keystore.c:29 emits [-Wcpp] and the build exits 0. See finding 1 below.
2	Medium — the EBLDR_PRODUCTION_KEY branch does not link	Untouched. Fixed in #122, verified there.	grep -rn ebldr_production_key over the tree returns exactly two hits at this head — the extern declaration at core/keystore.c:125 and the memcpy at :126. Nothing defines or generates it.
3	Low — stacking not stated in the PR body	Resolved (last review). The body's range is now stale again after the re-stack; see the note under "On the re-stack".

Findings

#	Severity	File:line	Finding	Recommended fix
1	Critical (P0)	core/keystore.c:114-127; .github/workflows/release.yml (all firmware jobs)	Carried forward unchanged — this is the commit that arms the exposure. Nothing in the diff is wrong; the severity is entirely about merge order, and it is restated here only because the head moved and the constraint has to travel with it. Re-verified at ef5b70d0 as recorded in the table above. Do not merge #116 into master without #122 in the same batch, and do not tag a release from a master that has this without #122. The author has put this constraint at the top of the PR body, which is the right place for it.	Unchanged: land #115 → #116 → #122 as one batch, or land #122's configure-time FATAL_ERROR first. Nothing to change in this diff.
2	High (P1)	docs/quickstart.md:82-88; tools/sign_image.py:343,381; core/image_verify.c:176-178	New. The only documented way to embed your own trust anchor produces a header nothing reads, and the stack does not fix it. docs/quickstart.md tells a developer to run sign_image.py --genkey, which "Writes keys/private.pem, keys/public.pem and keys/public_key.h". That header defines ebldr_default_pubkey. But core/image_verify.c:176-178 records in its own comment that "ebldr_default_pubkey was never defined anywhere, so this did not link", and the code was changed to read the key from the keystore instead — so ebldr_default_pubkey now has zero consumers in the tree (grep finds it only in the two sign_image.py emitters and in that comment). The keystore's production slot expects a differently named symbol, ebldr_production_key. Net effect at this head: a developer follows the documented procedure, gets a public_key.h, builds, sees no error, and ships firmware whose trust anchor is still the RFC 8032 §7.1 test key — whose private half is printed in a public IETF document. .ai/security.md: "A target without hardware support degrades to a documented, weaker posture — it does not silently pretend to have the strong one." Here the documentation asserts the strong posture. This is distinct from finding 1: finding 1 is the release pipeline, this is the hand-build path an integrator is told to use. I checked whether #122 closes it: it does not. #122 adds cmake/ProductionKey.cmake, include/eos_production_key.h and rewrites docs/key_lifecycle.md, and leaves docs/quickstart.md:86 untouched — so after the whole stack lands the repo documents two different key paths, one of which does nothing.	Two small pieces, neither of which needs to be in this PR: (a) in docs/quickstart.md, stop listing public_key.h as an output that matters and point at the EBLDR_PRODUCTION_KEY / docs/key_lifecycle.md path that #122 establishes; (b) in tools/sign_image.py, either stop emitting public_key.h or emit it under the name the build actually consumes. Best landed on top of #122, since (a) should reference what #122 creates.

Beyond finding 2 I looked for a defect in the re-stacked commits and found none — there is no
new source on this branch since the last review.

Verification performed for this review

Detached worktree at .ai/autoreview/state/scratch/eBoot-116 off refs/pull/116/head. The
user's eBoot checkout (on fix/ed25519-low-order-keys) was not touched; nothing was
committed or pushed.

Check	Result
Is this head new work or a re-stack?	RE-STACK, confirmed. git patch-id --stable over this PR's own two commits (547ab9f, ef5b70d) is identical to the reviewed pair (7370d73, 31f44fb). The parent moved from a276016 to e152d8e — the same #115 commits after their own rebase. No content changed.
cmake -B build/host -DEBLDR_BUILD_TESTS=ON -DCMAKE_BUILD_TYPE=Release → cmake --build --parallel 4	PASS, no errors
ctest --test-dir build/host --output-on-failure --no-tests=error	PASS — 31/31, 2.56s
eboot_test_keystore verbose	PASS — 10/10, including test_compiled_in_anchor_verifies_its_own_rfc_vector
Does the #warning fail the build?	NO — confirmed by running it. core/keystore.c:29 emits under [-Wcpp], the build exits 0, and the message is one of 3 warnings in the log. This is the evidence for "the sole guard is advisory" in finding 1, which previous reviews stated from reading the source.
EBLDR_PRODUCTION_KEY wired at this head?	NO — zero hits in CMakeLists.txt or .github/.
ebldr_production_key defined at this head?	NO — two hits, both in core/keystore.c:125-126.
Board blast radius	CONFIRMED, all boards — grep -rln otp_read boards/ = 0 hits across 82 board directories.
Previous review's open "Not checked" item: do tools/sign_image.py / tools/eos_sign.py default to this same key?	ANSWERED — NO, and the answer produced finding 2. sign_image.py --genkey generates a fresh random Ed25519 keypair (tools/sign_image.py:300-350); it does not embed the RFC key. So a developer signing locally would not notice the anchor change — their images simply fail to verify. The problem it exposed instead is that the header --genkey emits (ebldr_default_pubkey) has no consumer anywhere in the tree.
Does #122 close finding 2?	NO — checked directly. git show a50df28:docs/quickstart.md still carries the public_key.h line verbatim, and #122's 16-file diff does not include docs/quickstart.md or tools/sign_image.py.
Negative control on test_compiled_in_anchor_verifies_its_own_rfc_vector	NOT RE-RUN. It was executed at 31f44fb8 and failed as designed at the memcmp. The commit is patch-identical here, so that result carries; I did not re-break the bytes.

On the re-stack. 31f44fb8 is no longer reachable, so the PR body's "review
a276016..31f44fb for this change alone" range no longer resolves — the equivalent range at
this head is e152d8e..ef5b70d. Not raised as a finding: the author already corrected one
stale range on this PR with the correction left visible, the ranges go stale on every re-stack
of a three-deep stack, and chasing them is not a good use of anyone's time. Noted so the next
reader is not confused by it. The same applies to "pytest 80 passed", which is 82 at #115's
current head for reasons unrelated to this branch.

Was anything weakened? No. No new source since the last review; ctest is 31/31 and
eboot_test_keystore 10/10, both unchanged. No test disabled, no assertion removed, no lint
loosened, no permission widened.

Architecture conformance

Conforms. §21: eBoot is Tier 1 Foundation; core/keystore.c, tests/unit/ and
docs/quickstart.md are all inside the owning repo. No #include, link line, CMake
target_link_libraries entry or manifest dependency changes at this head, so §5.1's
dependency direction is untouched and nothing points up a tier. §14.1 ("Integrate key
management across eBoot, eSec, eOTA and release signing") is the clause both findings sit
under — and finding 2 is precisely a failure to review that key management as one system:
the signing tool, the C build and the quickstart each describe a different anchor path, and
two of the three lead nowhere. §5.1's "eBoot keeps the trusted computing base minimal and
auditable" is engaged the same way: an anchor a developer believes they replaced, and did
not, is not auditable. The §14.1 master-design proposal asking that a release build be
required to carry a production anchor was appended by an earlier review of this PR and
stands; the 2026-09-14 proposal "A development trust anchor must be structurally unable to
reach a release artifact" in proposals/2026-09.md covers the same ground. No new proposal
is appended for finding 2 — this is a repository documentation defect, not a gap in the
master design, which already says enough in §14.1 to make it wrong.

Proposed changes

Merge order — unchanged, the sequencing IS the mitigation:
  1. #115
  2. #116 (this PR)  ]  land together, or land #122's CMake guard first.
  3. #122            ]  Do not tag a release from a master that has 2 without 3.

Nothing to change in this diff.

Follow-up on top of #122 (finding 2), smallest sequence:
  docs/quickstart.md   drop "and keys/public_key.h" from the --genkey output list;
                       point at docs/key_lifecycle.md + EBLDR_PRODUCTION_KEY for
                       embedding an anchor
  tools/sign_image.py  stop writing public_key.h, or write the symbol the build
                       actually consumes

Not checked

• No fix PR opened for finding 2. It is rated High, but the fix is documentation plus a
  tool-output change, and the brief's bar for an unattended fix PR is "small and provable —
  a fix you can verify by running something". There is no command that proves a doc sentence
  correct, and the right text depends on #122's docs/key_lifecycle.md, which is not merged.
  Recorded for a human instead.
• No released artifact was inspected. Finding 1 is still derived from reading
  release.yml, core/keystore.c, hal/hal_core.c and the boards/ tree — not from
  downloading a published .efw and grepping it for d75a9801….
• Cross-compile (STM32F4, Cortex-M4), fuzz-harness build, CodeQL, cppcheck/clang-tidy —
  NOT RUN locally. CI green on this head; job logs not read.
• pytest tests/ — NOT RUN at this head. The two commits touch no Python. I ran it at
  #115's current head (82 passed); not executed at this SHA.
• ASan/UBSan and Valgrind — NOT RUN.
• Negative control — NOT RE-RUN at this head, see the table. The earlier result is relied
  on because the commit is patch-identical, which is Inferred from patch-id, not re-observed.
• RFC 8032 was not re-fetched. Curve membership and the byte-21 divergence were
  established by computation in the first review and are not redone here.
• I did not check whether docs/quickstart.md's signing flow otherwise still works —
  finding 2 is about the public_key.h line specifically, not about imgpack.py/sign_image.py
  end to end, which I did not execute.

---

Automated architecture review of ef5b70d0f702 — scheduled, model claude-opus-5, checked against the EmbeddedOS Master Design v2.0. Advisory only: this reviewer never approves, requests changes, or merges. Reply here to discuss or push back — a wrong finding is a bug worth reporting.

[Kartikey1306]

Answering the 06:44Z re-review at ef5b70d. No content change on this branch — a pure restack, as you found by patch-id — so the carried findings stand as sequenced:

• Finding 1 (P0, sequencing): unchanged and unchangeable here; the body's first paragraph states the constraint in the merger's terms (land with build: a Release build of a real board refuses to configure without a production trust anchor #122, or land build: a Release build of a real board refuses to configure without a production trust anchor #122's gate first; do not tag from a master with this and not that).
• Finding 2 (P1, NEW) — the documented key path writes a header nothing reads. Taken, on top of build: a Release build of a real board refuses to configure without a production trust anchor #122 as you recommended: fix(sign_image): --genkey writes the key the build reads, not a header nothing includes #142 (fix/genkey-emits-the-key-the-build-reads, 0c7c8f5), closing quickstart's key-generation path writes a header nothing reads; firmware built by the documented procedure keeps the RFC test key #141. Both --genkey and --extract-pubkey now write public_key.hex — the exact EBLDR_PRODUCTION_KEY value — and print the cmake line; the header is not written under any name, since a second mechanism for the same thing is how this drifted. docs/quickstart.md shows the configure step. Four tests; the written value is asserted to pass the same check_production_key.py the gate runs, so the documented path cannot produce a key the build refuses. Negative controls 3/20 and 1/1. End to end at 0c7c8f5: --genkey → configure with the hex → generated/production_key.c carries exactly the generated key, byte-compared.
• Finding 3 (Low, range): the body's range line was corrected to e152d8e..ef5b70d with the old range named, at 12:10 IST — four minutes before this review posted, so it may have read the pre-edit body. It resolves now.

Nothing pushed here; nothing to re-approve.