build: a Release build of a real board refuses to configure without a production trust anchor

[Kartikey1306]

Stacked on #116 → #115. Review ef5b70d..18fab20 for this change alone: 13 commits, 16 files. (Range updated after 18fab20: one sentence in docs/key_lifecycle.md — the typo-collision rate is now the derivable 1/16 with three keys' measurements cited, per the review at a50df28; corrected from ef5b70d..a50df28.) (Corrected from 31f44fb..1cd1265 at 12:15 IST 2026-09-15: the stack was restacked base-first onto master 682d005 — #115 a276016→e152d8e, #116 31f44fb→ef5b70d, this branch 1cd1265→a50df28; twelve commits replayed clean, content unchanged, verified on the new head before the push: build OK, ctest 32/32, pytest 110 passed / 3 skipped, 17 workflows parse. Range updated after 1cd1265: the scanner now reads .obj as well as .o — the cross-target object naming the proof step had already learned — and docs/key_lifecycle.md states the ~1-in-20 typo case no key check can catch. Corrected from 31f44fb..4d4e702; earlier corrections in the history below.)

Problem (eBoot #120, raised as P0 in the review of #116)

core/keystore.c falls back to a compiled-in public key when the board has no OTP. Verified: no board under boards/ implements otp_read or otp_write (grep -rl otp_read boards/ is empty), so on every shipped board the compiled-in key is the trust anchor. And:

• EBLDR_PRODUCTION_KEY had no CMake option and nothing set it, so every firmware job in release.yml compiled the #ifndef branch — the RFC 8032 §7.1 TEST 1 public key, whose secret is printed in the RFC.
• The #else branch declared extern const uint8_t ebldr_production_key[] and nothing in the tree defined it: following the #warning's own instruction produced a link error.
• The only guard was that #warning, which fails nothing and is invisible in a release log.

Until #116 the anchor was off the curve and inert; #116 makes it work, which makes this live. .ai/security.md: "Test keys and development keys must be structurally incapable of signing a release artifact." They were not.

What this does

Build (CMakeLists.txt, cmake/ProductionKey.cmake, include/eos_production_key.h, core/keystore.c). -DEBLDR_PRODUCTION_KEY=<64 hex characters> is checked — exactly 64 hex, and not the development key — then turned into build/generated/production_key.c defining ebldr_production_key[], compiled into eboot_core, and EBLDR_PRODUCTION_KEY=1 selects the production branch of keystore.c. A Release build of a real board that sets no key refuses to configure:

CMake Error at CMakeLists.txt:56 (message):
  EBLDR_PRODUCTION_KEY is not set: a Release build of board 'stm32f4' would
  compile in the RFC 8032 test key as its trust anchor, and anyone can sign
  for that key. Pass -DEBLDR_PRODUCTION_KEY=<64 hex characters> (the raw
  Ed25519 public key), or, for a bring-up or CI build that will never reach a
  device, -DEBLDR_ALLOW_DEV_KEY=ON.

-DEBLDR_ALLOW_DEV_KEY=ON is the one, explicit way past it. Host and Debug builds are not gated; development is unchanged apart from the existing #warning.

Release workflow. All 8 board configures in release.yml pass -DEBLDR_PRODUCTION_KEY="${{ secrets.EBLDR_PRODUCTION_KEY_HEX }}". With the secret unset the value is empty and the configure fails closed — a release cannot be cut on the development key until a maintainer adds that secret; that is the intended shape, and the comment at the first use says so. After each of the 6 firmware builds, a step scans every .elf/.bin/.a/.o under build/ for the development key's 32 bytes and fails on a hit. ci.yml's ARM cross-compile is Release + stm32f4 and passes the opt-out, with a comment.

Tests.

• tests/unit/test_keystore_production.c — compiles core/keystore.c under EBLDR_PRODUCTION_KEY with a fixture key (RFC 8032 TEST 2, generated by the same CMake function the real build uses). The production branch of the keystore — the one a release is built from — is built and run on every host build for the first time. It links eboot_core for eos_crypto_hash(); its own keystore.o defines every keystore symbol, so the archive's copy is never pulled (nm: default_dev_key absent from the binary).
• tests/unit/test_production_key_gate.py — real configures against the tree: Release+board with no key fails with the gate message as the first error; the opt-out clears it; Debug and host builds are not gated; the development key (both cases) and four malformed values are refused; a real key generates the anchor with exactly those bytes.
• tests/unit/test_release_workflow_production_key.py — all 8 board configures carry the secret and none the opt-out; all 6 firmware jobs scan after building and before collecting; the bytes the scan names equal what core/keystore.c and cmake/ProductionKey.cmake name; and no workflow anywhere cross-compiles a Release board without a key or the opt-out.

Docs. docs/key_lifecycle.md §3.1/§7.2/§7.4 described a header scheme (eos_signing_key.h, EOS_BUILD_PRODUCTION) that does not exist and a development key that was not the one compiled in; they now describe what the tree does. CHANGELOG Security entry.

Verification

Verified, macOS/clang, from a clean build directory:

Check	Result
Release + stm32f4, no key	CMake Error … EBLDR_PRODUCTION_KEY is not set as the first error, rc≠0
same + -DEBLDR_ALLOW_DEV_KEY=ON	no gate message, configure rc 0
-DEBLDR_PRODUCTION_KEY=<dev key>	refused: "the development key, whose secret is published"
-DEBLDR_PRODUCTION_KEY=abc	refused: "exactly 64 hexadecimal characters"
host build, -DEBLDR_PRODUCTION_KEY=<RFC TEST 2>	eboot_core builds, keystore #warning absent, nm shows ebldr_production_key and no default_dev_key
ctest (Release) / (Debug + ASan/UBSan)	32/32 and 32/32
pytest tests/	92 passed
scan snippet against a file containing the dev bytes / a clean file	hit, exit 1 / exit 0
Negative controls	fixture key set to the dev key → CMake refuses; one fixture byte flipped → test_keystore_production fails; drop the flag from one esp32c3 configure / change one scan hex digit / remove ci.yml's opt-out → the workflow guard fails each time

Not verified here: an actual tag release (the secret does not exist yet; the release will fail closed at configure until it does — see below) and cross-compiles (no toolchains on this machine).

For the maintainers

1. release.yml will fail closed until the EBLDR_PRODUCTION_KEY_HEX repository secret exists — the raw Ed25519 public key as 64 hex characters (§2.2 of docs/key_lifecycle.md). That is by design.
2. eos's eos-simulation.yml and upstream-drift.yml build eBoot in Release for qemu_arm64 and would trip the gate; embeddedos-org/eos PR (ci: opt the eBoot builds into the development key) adds the opt-out there and is safe to land first.

Closing issue

Fixes #120

[codecov-commenter]

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

Codecov Report

❌ Patch coverage is 90.81836% with 46 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
tools/check_no_dev_anchor.py	61.70%	22 Missing and 14 partials ⚠️
tools/check_production_key.py	92.50%	3 Missing and 3 partials ⚠️
tests/unit/test_fw_update_test_sigs.py	93.33%	1 Missing and 1 partial ⚠️
tests/unit/test_release_workflow_production_key.py	98.90%	1 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

[srpatcha]

Review — eBoot#122 "build: a Release build of a real board refuses to configure without a production trust anchor"

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

Verdict: This is the structural fix the #116 review asked for, and the core of it works —
I built it both ways and the artifact guard fires on a dev-key build and is silent on a
production one. Two gaps, both measured, both in the gate rather than the mechanism: the
configure gate recognises exactly one spelling of "release" and misses MinSizeRel, and the
key it accepts is never checked for being a point on the curve — the precise defect #116 just
spent a PR fixing, now reachable through a typo in the release secret. Neither is hard to
close and both should be closed before this merges, because this PR is what makes the anchor
load-bearing.

Findings

#	Severity	File:line	Finding	Recommended fix
1	High (P1)	CMakeLists.txt:54-55; docs/key_lifecycle.md:343	The gate matches the literal string Release and nothing else. I ran real configures of a real board (stm32f4) at this head. Release → refused, correctly. RelWithDebInfo, MinSizeRel, release, RELEASE, and an unset CMAKE_BUILD_TYPE → all five configure successfully, exit 0, and print -- Trust anchor: RFC 8032 test key (development only; see core/keystore.c) as a STATUS line. MinSizeRel is the ordinary build type for a bootloader; -DCMAKE_BUILD_TYPE=MinSizeRel is not a suspicious thing for an integrator to type, and it silently produces a device image trusting a key whose secret is in an IETF document. Inside this repo release.yml is backstopped by the artifact scan (finding 3 notwithstanding, I verified it catches this), so the published artifacts are covered — but the gate is the only control for anyone building a device image outside this workflow: a vendor, a downstream fork, a board bring-up that becomes a product. docs/key_lifecycle.md:343 states the guarantee absolutely — "Development key is never used in production" — and that sentence is not true for four of the five spellings above.	Normalise before comparing, and treat every optimised build type as release-shaped: string(TOUPPER "${CMAKE_BUILD_TYPE}" _bt) then gate on _bt MATCHES "^(RELEASE|RELWITHDEBINFO|MINSIZEREL)$". Consider gating an unset build type too — a board build with no build type is not obviously a development build. Add the cases to tests/unit/test_production_key_gate.py, which today only parametrises Release and Debug, and align is_release_shaped() in test_release_workflow_production_key.py:57 with whatever the gate ends up matching, or the two will drift apart. Then soften key_lifecycle.md:343 to describe what the gate actually covers.
2	High (P1)	cmake/ProductionKey.cmake:23-37	ebldr_check_production_key_hex() never checks that the key is a point on the curve. It validates length, hex-ness, and "is not the dev key" — nothing else. I configured a Release stm32f4 build with -DEBLDR_PRODUCTION_KEY=d75a…4377725, which is the exact off-curve byte string #116 removed from core/keystore.c, and it was accepted: -- Trust anchor: production key from EBLDR_PRODUCTION_KEY, configure rc 0. I confirmed independently that those bytes decode to no point on edwards25519. One mistyped hex digit in the EBLDR_PRODUCTION_KEY_HEX secret therefore ships a fleet whose bootloader refuses every firmware image it is ever offered, with a green build, a green artifact scan, and a status line saying the production key is in place. That is #116's bug re-entering through the door this PR just built, and #116 exists because nothing ever checked. It fails closed — devices refuse firmware rather than accept forgeries — so this is reliability, not compromise; it is still the worst outcome short of compromise, because it is unrecoverable in the field.	CMake cannot reasonably do the field arithmetic, so put the check where the rest of the key policy already lives in Python: extend tests/unit/test_production_key_gate.py with a decode-and-recover-x test, and — more useful — have release.yml validate secrets.EBLDR_PRODUCTION_KEY_HEX decodes to a curve point in a step before the build, so a bad secret fails the release rather than the fleet. A ~15-line pure-Python check (recover x²=(y²−1)/(dy²+1), assert a square root exists) is enough and needs no dependency.
3	Low	.github/workflows/release.yml — the six Refuse an artifact that embeds the development anchor steps	The scan's file list and the artifact list disagree, and the scan is copy-pasted six times. The scan inspects .elf .bin .a .o; Collect artifacts ships .hex and .uf2 as well (:80) and .efi (:341). This is not exploitable today and I checked rather than assumed: any build that embeds the key leaves the bytes in build/**/keystore.c.o and libeboot_core.a, which the scan does read — that is exactly where my dev-key build was caught. The gap is that the two lists are maintained separately and only one of them is pinned by a test, so a future job that ships a .hex produced from pre-built objects, or prunes objects before the scan, loses coverage without anything going red. Separately, fifteen lines of security check exist verbatim in six places; test_the_scan_looks_for_the_key_keystore_actually_compiles_in pins the key in every copy, which is the important half, but not the suffix list or the exit logic.	Lift the scan into tools/check_no_dev_anchor.py (or a composite action) and have each job call it, so there is one copy to keep right. Derive its suffix list from, or assert it covers, the Collect artifacts globs.

Not a finding, recorded because it reads like one: tests/CMakeLists.txt:227 uses RFC 8032
§7.1 TEST 2's public key as the production fixture — another published key with a published
secret. That is correct for a host test binary that never ships, and the gate rejects only
TEST 1, so the value would also be accepted as a real production key. Combined with finding 2
I would rather the validation grew than the blocklist; a blocklist of published keys is not a
winnable game and I am not proposing one.

What this PR gets right

Worth stating plainly, because the findings above are about the edges and the middle is sound.
test_keystore_production is the answer to #116's finding 2 done properly: rather than assert
the production branch exists, it compiles core/keystore.c a second time with
EBLDR_PRODUCTION_KEY=1 and a generated fixture key, so the branch that only a real device
takes is built and executed on every host build — and test_no_otp_board_does_not_get_the_development_key
asserts the negative directly. The target_link_libraries(… eboot_core) comment explaining
why the archive's copy is not pulled in is exactly the kind of thing that stops a future
reader "fixing" the duplicate compile. test_release_workflow_production_key.py parses
release.yml rather than grepping it, asserts the scan sits between build and collect,
cross-checks that the bytes the scan hunts for are the bytes core/keystore.c actually
compiles in and the bytes ProductionKey.cmake blocks, and its last commit adds a synthetic
workflow so the guard's own branches execute on a green tree instead of being dead code that
has never been observed to fire. That last move is the difference between a check and a check
that works.

Verification performed for this review

Detached worktree at .ai/autoreview/state/scratch/eBoot-122 off refs/pull/122/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
ctest --test-dir build/host --output-on-failure --no-tests=error	PASS — 32/32, 11.01s (31 before this PR, +test_keystore_production)
eboot_test_keystore_production verbose	PASS — 3/3
pytest tests/ -q with EOS_REQUIRE_SIGNING_TESTS=1	PASS — 94 passed, 5.19s (80 before this PR, +14 from the two new modules)
Gate probe — real configures, EBLDR_BOARD=stm32f4, no key, no opt-out	Release → refused ✅ · RelWithDebInfo → rc 0, dev key ❌ · MinSizeRel → rc 0, dev key ❌ · release → rc 0, dev key ❌ · RELEASE → rc 0, dev key ❌ · unset → rc 0, dev key ❌. Finding 1.
Off-curve key probe — -DEBLDR_PRODUCTION_KEY= the bytes #116 deleted	ACCEPTED, Trust anchor: production key from EBLDR_PRODUCTION_KEY. Independently confirmed off-curve by solving x²=(y²−1)/(dy²+1) mod 2²⁵⁵−19 — no square root. Finding 2.
Artifact guard, end to end. Ran release.yml's scan verbatim against two real builds of x86_64_efi, CMAKE_BUILD_TYPE=Release	With -DEBLDR_PRODUCTION_KEY=<TEST 2>: clean, exit 0, no artifact contains the dev bytes. With -DEBLDR_ALLOW_DEV_KEY=ON: exit 1, hits on CMakeFiles/eboot_core.dir/core/keystore.c.o and libeboot_core.a. The guard is real in both directions.
Does a production build still carry default_dev_key?	NO — it is inside #ifndef EBLDR_PRODUCTION_KEY, so it is not compiled at all. The scan result above confirms it.
False-positive risk from test objects in a release build	NONE — CMakeLists.txt:23 defaults EBLDR_BUILD_TESTS to OFF, so tests/vectors/fw_update_test_sigs.h's copy of the same key never reaches a release build tree. I checked because a spurious red release would get the guard disabled within a week.
All 8 release board configures pass the key; none passes EBLDR_ALLOW_DEV_KEY	CONFIRMED by reading the diff, and pinned by test_every_release_workflow… which asserts seen == 8.
ci.yml:106 ARM cross-compile opts out explicitly	CONFIRMED — -DEBLDR_ALLOW_DEV_KEY=ON with a comment saying why. Correct: that job proves the tree cross-compiles and ships nothing.

Architecture conformance

Conforms. §21: eBoot is Tier 1 Foundation; every file is inside the owning repo, and the new
cmake/ and include/eos_production_key.h sit where .ai/architect.md's target shape puts
them — include/ is the contract, and it depends on nothing but eos_keystore.h and
<stdint.h>. §5.1 dependency direction is untouched; the generated translation unit is a leaf
compiled into eboot_core and nothing links upward. §5.1's "eBoot keeps the trusted computing
base minimal and auditable" is the clause this PR serves most directly: a trust anchor that a
build could silently substitute was not auditable, and a configure that refuses to proceed is
auditability expressed as a build failure. §14.1 ("Integrate key management across eBoot, eSec,
eOTA and release signing") is satisfied on the eBoot side; the §14.1 proposal appended during
the #116 review — that the master design should require a release build to carry a
production anchor — is exactly what this PR implements, and it stands unchanged. §28's status
policy is honoured: EBLDR_PRODUCTION_KEY is no longer a planned capability described in the
present tense, because the branch now compiles and runs in CI.

Proposed changes

Before merge, in this PR:
  CMakeLists.txt:54     string(TOUPPER "${CMAKE_BUILD_TYPE}" _bt)
                        elseif(_bt MATCHES "^(RELEASE|RELWITHDEBINFO|MINSIZEREL)$" ...)
  test_production_key_gate.py   parametrise the four spellings above + unset
  release.yml           validate the secret decodes to a curve point, before the build
  key_lifecycle.md:343  state what the gate covers, not "never"

After merge, not blocking:
  tools/check_no_dev_anchor.py  one copy of the artifact scan, called by six jobs

Merge order: #115 -> #116 -> #122. Do not land #116 without this one
(see the #116 review) — #116 arms the anchor, this is what disarms it.

No fix PR opened. Both High findings are in code that exists only on this branch —
CMakeLists.txt's gate and cmake/ProductionKey.cmake are not on origin/master — so a fix
branched from the default branch, which is what fix-start.sh produces, would have nothing to
patch. These belong to the author, in this PR, before it merges.

Not checked

• release.yml was not executed. Every statement about it comes from reading the YAML and
  from running its scan step's Python verbatim against local builds. I did not trigger a tag,
  and I could not test the secrets.EBLDR_PRODUCTION_KEY_HEX-unset path end to end. Reading
  it, an unset secret expands to -DEBLDR_PRODUCTION_KEY="", which takes the gate branch and
  fails the configure — fail-closed, and the || fallback in the esp32 jobs then also fails,
  so cmake --build runs against an unconfigured directory and errors. That is Inferred,
  not Verified.
• Cross-compile, fuzz, CodeQL, cppcheck/clang-tidy — NOT RUN locally. CI green on this
  head; logs not read. In particular I did not verify that a real cross-compiled
  stm32f4/esp32 Release build with a production key links — my end-to-end artifact test
  used a host-toolchain x86_64_efi build. If ebldr_production_key[] fails to link on a
  cross target, CI's ARM job would not catch it because that job opts out with
  EBLDR_ALLOW_DEV_KEY=ON and so never compiles the production branch. That is a real
  coverage hole in the CI matrix and I could not close it from here — no cross toolchain on
  this host.
• ASan/UBSan and Valgrind — NOT RUN.
• Key rotation, HSM and slot-lifecycle content in docs/key_lifecycle.md (§§4-7, the
  rotation phases and custody table) was read for contradictions with the code and none were
  found, but it describes an operational process I cannot verify from the repository.
• No published artifact was downloaded and inspected. As with #116, the release-path
  reasoning is from source, not from a binary someone can download today.

---

Automated architecture review of 6a1d21f9f953 — 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]

Both P1s closed at 302fc5f, the way you specified, and verified by running the configures rather than reading the gate.

Finding 1 — the gate matched only the literal Release. I went one step past the regex you proposed: the gate now exempts only Debug (case-insensitive) and refuses everything else, including an unset build type. Reason: the cross-compile branch in this same file adds -Os regardless of build type, so an unset-type cross build is optimised — release-shaped — and a multi-config generator has no build type at all at configure time. Asking "is this spelled like a release?" was the original mistake; "is this Debug?" is the question with a finite answer. The error now names the build type it saw, or says there was none. Real stm32f4 configures at this head: Release, RelWithDebInfo, MinSizeRel, release, RELEASE, unset → all six refused; Debug/debug → configure; MinSizeRel + EBLDR_ALLOW_DEV_KEY=ON → configures; host build with no board → no gate. No CI leg is affected: ci.yml's ARM leg already passes the opt-out, and nightly/weekly cross builds set no board. Wrong, and CI said so within the hour: build.yml's Cross-compile STM32F4 job configures stm32f4 with no CMAKE_BUILD_TYPE and no opt-out — exactly the unset case the gate now refuses — and went red at 302fc5f. I had grepped for lines that set a build type and not for jobs that set none. The gate was right; f011dc5 gives that job the same -DEBLDR_ALLOW_DEV_KEY=ON (with the never-in-a-publishing-workflow comment) that ci.yml's ARM leg already carries, and every other EBLDR_BOARD= in .github/ was then audited by grep: each carries Debug, the opt-out, or a production key.

Finding 2 — the key was never checked against the curve. tools/check_production_key.py, pure Python, no dependencies, applies core/ed25519_verify.c's own rule rather than a weaker one: decode per RFC 8032 §5.1.3, [L]P == identity, P != identity, and not the development key. So it refuses the off-curve bytes, the identity, all the low-order points, a non-canonical y ≥ p, and the dev key — each for a stated reason. Wired in three places, in the order you suggested plus one: release.yml runs it on the secret in the validate job that every firmware job needs, so a bad secret fails the release before a single board is configured; cmake/ProductionKey.cmake runs it at configure when python3 is found and, when it is not, warns naming exactly what was not checked rather than staying silent; and tests/unit/test_check_production_key.py pins 15 vectors with each refusal asserted on its reason — which caught two of my own first vectors being refused for length rather than for the branch they claimed to test. Your exact reproduction, -DEBLDR_PRODUCTION_KEY=d75a…4377725 on a Release stm32f4: refused, "the bytes decode to no point on edwards25519", nothing generated.

On the TEST 2 fixture you flagged as reading like a finding: agreed it is correct for a host binary that never ships, and the tool now accepts it for the right reason — it is a point in the subgroup — rather than because the blocklist happens not to name it. No blocklist grew.

docs/key_lifecycle.md:343 no longer says "never"; it says what the gate covers and lists what it cannot see (a Debug build flashed to a device, an explicit opt-out, a fork that removes the gate).

Negative controls: gate reverted to the literal Release → 3 tests fail; curve-check result ignored → 3 tests fail. Restored. pytest 121 passed, host build clean, ctest 32/32. Body range updated to 31f44fb..302fc5f.

Not done, deliberately: finding 3 (the six copies of the artifact scan) is unchanged — a follow-up, as you rated it. And the CMake-side curve check depends on python3 being present; an integrator without it gets a WARNING that says so, not a refusal. I judged a hard python3 requirement on every production configure to be a bigger dependency change than this PR should make; if you disagree, it is one line.

[Kartikey1306]

Finding 3 is now closed, and one branch of the finding-2 fix is hardened. Head is 8d09efb; everything below was measured there.

Finding 3 — six inline scans → one tool, and the list drift was the smaller half

805f2bf adds tools/check_no_dev_anchor.py and has all six scan steps call it. The suffix-set test does what you asked — it parses every -name "*.x" glob out of every Collect artifacts step and asserts the tool's set covers them — so the two lists can no longer be maintained apart.

But extending the suffix list turned out to be the wrong fix on its own. Intel HEX is ASCII and UF2 is 512-byte blocks with headers, so grepping either for the key's raw bytes finds nothing even when the key is in the image. A scan that opened .hex/.uf2 and reported them clean would be worse than the old one that never opened them. The tool decodes both to the image they encode and merges records/blocks into contiguous runs, so a key straddling a 16-byte HEX record or a 256-byte UF2 block is one search. A file with a scanned suffix that cannot be decoded is a failure, not a skip — a file the scanner could not read is a file it did not check.

The test for this asserts the premise before the conclusion: a raw search of the fixture .hex and .uf2 returns False, then the tool finds the key in both. Without the first assertion the second could pass for the wrong reason.

is_release_shaped() in the workflow test still matched the literal Release; it now mirrors the gate 302fc5f left (any optimised type, case-insensitive, or none named), which was your alignment note on finding 1.

Finding 2 — the no-python3 branch

302fc5f runs the point check through execute_process and, when python3 is not found, emitted a WARNING and continued — the key compiled in unchecked. Your own framing of finding 2 is why that is the wrong branch: the gate is "the only control for anyone building a device image outside this workflow", and a warning scrolls past. 8d09efb makes it FATAL_ERROR, naming what to install. Development builds pass no key and never reach it.

Negative controls, reverting one piece and keeping the tests

scanner grepping .hex/.uf2 raw instead of decoding   ->  2 of 9 fail
scanner suffix set missing .hex                       ->  3 of 9 fail
one inline scan restored in release.yml               ->  1 of 9 fail
WARNING branch restored in ProductionKey.cmake        ->  1 failed

At 8d09efb

$ python -m pytest tests/ -q
126 passed

$ ctest --test-dir build --no-tests=error
100% tests passed, 0 tests failed out of 32

production key + -DCMAKE_DISABLE_FIND_PACKAGE_Python3=TRUE  -> rc 1, "python3 was not found", no CMake Warning
production key, python3 findable                             -> rc 0
Debug board, python3 unfindable                              -> rc 0

All 17 workflows parse; release.yml's six scan steps and every firmware job's needs: validate are asserted from the parsed structure, not the text.

NOT RUN: the release workflow itself (needs a tag); the scan step runs the same script the tests pin. ARM cross-compile leg (no arm-none-eabi here).

For the merger

Merge order is unchanged: #115 → #116 → #122. The body's review range is now 31f44fb..8d09efb, corrected from 31f44fb..f011dc5. Your approval, if one lands on f011dc5, is dismissed by this push; 8d09efb is the head — could you re-approve. I'll hold further pushes to this branch.

[srpatcha]

Review — eBoot#122 "build: a Release build of a real board refuses to configure without a production trust anchor"

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

Verdict: Follow-up review of 6a1d21f9..8d09efbd (4 commits). All three findings from the
previous review are resolved, and I re-ran the probes rather than reading the gate — the
configure gate now refuses every non-Debug build type including an unset one, the key is
checked against the verifier's own subgroup rule before it is compiled into anything, and the
six inline artifact scans are one tool that decodes .hex/.uf2 instead of grepping them. The
new commits introduce no defect I could find. Three residual items below, none of them blocking:
one CI-coverage gap the opt-out in f011dc5 makes structural, and two small robustness/diagnostic
gaps in the new tools.

Previous findings — status

#	Previous finding	Status	Evidence
1	High (P1) — gate matched only the literal Release; RelWithDebInfo, MinSizeRel, release, RELEASE, unset all configured on the dev key	Resolved in 302fc5f	CMakeLists.txt:55-77 now uppercases CMAKE_BUILD_TYPE and exempts only DEBUG. Real configures of EBLDR_BOARD=stm32f4 at this head, no key, no opt-out: Release, RelWithDebInfo, MinSizeRel, release, RELEASE, unset → all six rc 1, refused; Debug, debug, DEBUG → rc 0. Inverting the question from "is this spelled Release?" to "is this Debug?" is the right call — it has a finite answer, and a multi-config generator has no build type at configure time at all.
2	High (P1) — ebldr_check_production_key_hex() accepted a key that is not a point on the curve	Resolved in 302fc5f, hardened in 8d09efb	cmake/ProductionKey.cmake:52-77 shells out to the new tools/check_production_key.py. Verified through CMake on a Release stm32f4: off-curve bytes → rc 1, "the bytes decode to no point on edwards25519", and build/generated/ is not created; an on-curve point outside the prime-order subgroup → rc 1; RFC 8032 TEST 2 → rc 0, production_key.c written. 8d09efb turns the no-python3 branch from WARNING into FATAL_ERROR — confirmed with -DCMAKE_DISABLE_FIND_PACKAGE_Python3=TRUE: rc 1, zero CMake Warning lines. I read core/ed25519_verify.c:321-334 against the Python: the rule is the same ([L]P == identity && P != identity), and where they differ the Python is the stricter side (it rejects y >= p and x == 0 with the sign bit set, which unpackneg() does not), so it cannot accept a key the verifier would refuse.
3	Low (P3) — six copies of the artifact scan, suffix list disagreeing with the artifacts collected	Resolved in 805f2bf	tools/check_no_dev_anchor.py replaces all six inline blocks (release.yml:71,104,136,183,228,257). The point the author raises is correct and is the more important half: extending the suffix list alone would have been worse than the old scan, because Intel HEX is ASCII and UF2 is 512-byte framed, so a raw grep of either reports clean on an image that does contain the key. Both are decoded and merged into contiguous runs first. I ran the scanner against two real x86_64_efi Release builds: production key → clean, rc 0; EBLDR_ALLOW_DEV_KEY=ON → rc 1, hits on CMakeFiles/eboot_core.dir/core/keystore.c.o and libeboot_core.a.

The TEST 2 fixture point recorded as "not a finding" last time is unchanged and still not a
finding; the tool accepts it because it is a point in the subgroup, and no blocklist grew.

Findings

#	Severity	File:line	Finding	Recommended fix
1	Medium (P2)	.github/workflows/build.yml:55; .github/workflows/ci.yml:114	After this PR, no pre-merge CI job compiles the production-key branch at all, on any target. f011dc5 is the correct fix for the job it unblocked, but it makes the pattern structural: EBLDR_PRODUCTION_KEY now appears only in release.yml (verified by grep over .github/workflows/), and both jobs that configure a real board — ci.yml's ARM leg and build.yml's Cross-compile STM32F4 — pass EBLDR_ALLOW_DEV_KEY=ON. So build/generated/production_key.c and the #ifdef EBLDR_PRODUCTION_KEY branch of core/keystore.c are first cross-compiled when a tag is pushed, inside the workflow that publishes. The previous review flagged this as something it could not close ("Not checked"); it is now the shape of the matrix rather than a gap in my coverage, which is why I am recording it as a finding. A link or section-placement problem in the generated TU would surface during a release, not during review. It is not newly created by this PR — build.yml's job previously compiled the dev branch — but this PR is what makes the production branch load-bearing.	Add one cross-compile leg that passes a fixture production key rather than the opt-out, e.g. in build.yml: -DEBLDR_PRODUCTION_KEY=3d4017c3…660c -DCMAKE_BUILD_TYPE=Release (RFC 8032 TEST 2, already used as the host fixture at tests/CMakeLists.txt:227) instead of -DEBLDR_ALLOW_DEV_KEY=ON. That compiles and links the branch that only a real device takes, on a cross toolchain, on every PR, and the artifact scan stays green because the dev key is then not compiled in at all. Follow-up is fine; it does not block this PR.
2	Low (P3)	tools/check_no_dev_anchor.py:67, :134	A malformed Intel HEX record escapes the handler as an IndexError traceback instead of the intended ::error annotation. decode_intel_hex() indexes rec[0..3] before checking the record has four bytes; scan() catches only ValueError and UnicodeDecodeError. Reproduced: a .hex file containing :00 gives a Python traceback. It still fails closed — an uncaught exception exits 1, so the job goes red — so this is presentation, not a hole: the operator loses the file-annotated "could not be decoded for the anchor scan" line the code intends, and the tool's own stated contract ("a file the scan could not read is a file it did not check") is then delivered by accident rather than by design. Separately, p.suffix not in SUFFIXES at :129 is case-sensitive, so a .BIN/.HEX artifact would be skipped and reported clean; not exploitable today because release.yml's find … -name "*.bin" globs are case-sensitive too, so the two lists still agree.	if len(rec) < 5: raise ValueError("Intel HEX record is truncated") before unpacking, and add IndexError and struct.error to the except at :134. Compare p.suffix.lower(). One test with a :00 fixture pins the first half.
3	Low (P3)	tools/check_production_key.py:116; tests/unit/test_check_production_key.py:37-41	"a point of low order" is reported for every point outside the prime-order subgroup, which is wrong for the case an operator will actually hit. A mistyped hex digit yields, ~50% of the time, a point on the curve of order 8L — not a low-order point. Verified: flipping the last digit of the dev key gives "the key is a point of low order". That message points a release engineer at "someone handed me an attack vector" when the true cause is "you typed the secret wrong", and it contradicts the module docstring at :13-14, which correctly describes the two distinct failure modes. The three parametrised vectors are all genuine low-order points (orders 2, 4, 8), so no test covers the mixed-order case — which is why the wording was never challenged.	Reword to "the key is not in the prime-order subgroup of edwards25519 (a mistyped key usually lands here)", and add a vector for a point of order 8L — e.g. 0300…00, which I confirmed decodes to a curve point that the tool refuses. Adjust the "low order" assertions accordingly.

Verification performed for this review

Detached worktree at .ai/autoreview/state/scratch/eBoot-122 on refs/pull/122/head. The user's
eBoot checkout is on fix/ed25519-low-order-keys, was clean before and after, and was not
touched; nothing was committed or pushed. refs/autoreview/pr122 in the local clone still pointed
at 6a1d21f9, so prior-reviews.txt could not produce the incremental diff; I fetched
refs/pull/122/head into a scratch ref read-only to get it.

Check	Result
cmake -B build/host -DEBLDR_BUILD_TESTS=ON -DCMAKE_BUILD_TYPE=Debug → cmake --build --parallel 4	PASS
ctest --test-dir build/host --output-on-failure --no-tests=error	PASS — 32/32, 11.03s
EOS_REQUIRE_SIGNING_TESTS=1 pytest tests/ -q	PASS — 126 passed, 9.65s (94 at the previously reviewed head)
Gate probe — real configures, EBLDR_BOARD=stm32f4, no key, no opt-out	Release / RelWithDebInfo / MinSizeRel / release / RELEASE / unset → all rc 1, refused ✅ · Debug / debug / DEBUG → rc 0, dev key ✅
Gate probe — -DEBLDR_ALLOW_DEV_KEY=ON on Release stm32f4; host build, no board	rc 0, dev-key status line — both correct
Key check through CMake — off-curve 0200…00; on-curve non-subgroup 0300…00; dev key; TEST 2	refused / refused / refused / accepted, generated/production_key.c written only in the accepted case
Key check — python3 unfindable with a valid key	rc 1, FATAL_ERROR, 0 CMake Warning lines — the 8d09efb change does what it claims
check_production_key.py direct vectors	dev key, identity 01·00³¹, order-4 00³², y = p, 4-char input, non-hex → all refused with distinct reasons; TEST 2 → accepted
Empty secret (KEY="", the unset-secret path in release.yml:32)	rc 1, "must be exactly 64 hexadecimal characters; got 0" — fails closed
release.yml job graph, parsed with PyYAML	all six firmware-* jobs needs: validate; all six call check_no_dev_anchor.py; validate holds the key check and no board build
Artifact scanner end to end, two real x86_64_efi Release builds	production key → clean, rc 0 · EBLDR_ALLOW_DEV_KEY=ON → rc 1, hits on keystore.c.o and libeboot_core.a
Scanner robustness probes	truncated .hex record → traceback, rc 1 (finding 2) · .BIN containing the key → reported clean (finding 2)
core/ed25519_verify.c:321-334 vs check_production_key.py:95-117	Same acceptance rule. Python is strictly stronger on encoding canonicality, so no key it accepts can be one the C refuses.
Secret handling in release.yml:28-33	Correct pattern — the secret is bound through env: and referenced as "$KEY", so it is not interpolated into the logged script body.

Architecture conformance

Conforms; unchanged from the previous review and re-checked against the new files. §21: eBoot is
Tier 1 Foundation, and tools/, cmake/ and tests/unit/ all sit inside the owning repo — §21.1
is not engaged, nothing earned a new repository. §5.1 dependency direction is untouched: the two
new tools are build-time and test-time only, are not linked into any image, and nothing points up
a tier. §5.1's "eBoot keeps the trusted computing base minimal and auditable" is what this PR
serves — and 805f2bf serves it twice over, because six hand-maintained copies of a security
check were themselves an auditability defect, and a scan that opened .hex without decoding it
would have been an auditability defect that reported success. §14.1 ("integrate key management
across eBoot, eSec, eOTA and release signing"; "use reviewed cryptographic libraries, do not
invent primitives") is satisfied: check_production_key.py does not implement verification, it
re-applies the shipped verifier's stated acceptance rule at configure time, and I checked the two
agree. §28's evidence policy is honoured — the author's claims were reproducible, and the two that
were wrong in the previous round (f011dc5's CI breakage) were retracted in the thread rather
than left standing.

Proposed changes

Not blocking this PR. In order of value:

  build.yml               one cross leg with -DEBLDR_PRODUCTION_KEY=<TEST 2 fixture>
                          -DCMAKE_BUILD_TYPE=Release instead of -DEBLDR_ALLOW_DEV_KEY=ON,
                          so the production branch is cross-compiled before a tag   (finding 1)

  check_no_dev_anchor.py  len(rec) < 5 guard; catch IndexError/struct.error;
                          p.suffix.lower()                                          (finding 2)

  check_production_key.py "not in the prime-order subgroup" wording + an order-8L
                          vector in test_check_production_key.py                    (finding 3)

Merge order is unchanged: #115 → #116 → #122. The author has asked for re-approval on
8d09efb; from this side the two P1s that were the reason to hold it are closed and verified.

No fix PR opened. All three findings are in files that exist only on this branch
(tools/check_no_dev_anchor.py, tools/check_production_key.py) or in a line this branch
introduces (build.yml:55), so a branch cut from origin/master, which is what fix-start.sh
produces, would have nothing to patch. These belong to the author.

Not checked

• release.yml was not executed — it needs a tag. Every statement about it is from parsing
  the YAML with PyYAML and from running its steps' scripts verbatim against local builds. The
  secrets.EBLDR_PRODUCTION_KEY_HEX-unset path was tested only as KEY="" against the script,
  not end to end through Actions.
• Cross-compile, fuzz, CodeQL, cppcheck/clang-tidy — NOT RUN locally. CI is green on this head;
  I did not read the logs. Finding 1 is precisely about what that green does not cover.
• ASan/UBSan and Valgrind — NOT RUN.
• The UF2 and Intel HEX decoders were exercised only through the PR's own fixtures and my
  malformed-record probe. I did not test them against a real vendor-produced .uf2 (no board
  here), so "decodes what a real toolchain emits" is Inferred from the format, not Verified. The
  notMainFlash UF2 flag (0x00000001) is not consulted; blocks carrying it would still be
  searched, which is the safe direction.
• No published artifact was downloaded and inspected. The release-path reasoning is from
  source, not from a binary anyone can download today.
• Key rotation, HSM and custody content in docs/key_lifecycle.md describes an operational
  process I cannot verify from the repository. Only the one row this PR changed (:343) was
  checked against the code, and it now matches the gate's behaviour.

---

Automated architecture review of 8d09efbd8b28 — 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]

All three taken, in one push. Head is 4a98951; every number below was measured there.

1 (Medium) — the production-key branch is now cross-compiled on every PR

build.yml's Cross-compile STM32F4 leg passes -DEBLDR_PRODUCTION_KEY=<RFC 8032 TEST 2> -DCMAKE_BUILD_TYPE=Release instead of the opt-out — the fixture you named, a genuine curve point the configure-time check accepts, whose secret is published, for a job whose output reaches no device. ci.yml's ARM leg still compiles the dev-key branch, so both halves of core/keystore.c are cross-compiled before a tag.

Rather than trust that the branch was taken, a new step proves it: generated/production_key.c exists, its .o was built, and the object carries the fixture key and not the dev key. Locally, host compiler standing in for arm-none-eabi: configure rc 0, generated TU present, eboot_core builds with 0 errors, production_key.c.o → fixture present, dev key absent.

2 (Low) — the scanner reports rather than raises, and reads .BIN

Both halves reproduced first: :00 gave the IndexError traceback you showed, and a .BIN containing the dev key was skipped and the tree reported clean — that second one is the worse half. Now a length check before unpacking (and the record's own byte count is checked against its actual length), IndexError/struct.error caught with the rest, p.suffix.lower(). Two tests pin it: a :00 fixture must land in undecodable with "truncated", and FW.BIN + FW.HEX must both be found.

3 (Low) — low-order and mixed-order are told apart

Your 0300…00 and the dev key with its last digit changed both said "a point of low order". They now say "on the curve but not in the prime-order subgroup — a mistyped hex digit usually lands here; the verifier refuses it and every image would be rejected", and the order-2/4/8 vectors still say low order with "every signature would verify against it". The split is [8]P == identity, so it is computed, not a second blocklist. Two mixed-order vectors added, plus a test that asserts the two classes get different sentences — that was the missing case that let the wording stand.

Negative controls, one piece reverted at a time, tests kept

tools/check_no_dev_anchor.py at 8d09efb   ->  2 of 11 fail   (truncated record, .BIN)
tools/check_production_key.py at 8d09efb  ->  3 of 19 fail   (0300..00, mistyped dev key, class distinction)

At 4a98951

pytest tests/    131 passed
ctest            100% tests passed, 0 tests failed out of 32
17 workflows parse; build.yml's configure asserted from the non-comment lines of the
parsed step -- the opt-out flag is named in the comment explaining why it is gone,
which is the grep false-match worth stating

NOT RUN: the ARM cross-compile itself (no arm-none-eabi-gcc here). The proof step is what CI runs; what ran locally is the same configure and build with the host compiler.

For the merger

Order unchanged: #115 → #116 → #122. Body range now 31f44fb..4a98951 (correction chain visible). Any approval at 8d09efb was dismissed by this push; 4a98951 is the head — could you re-approve. I'll hold further pushes.

[Kartikey1306]

One more push, against what I said an hour ago — a red check is the exception, and this is the smallest one possible. Head is 4d4e702.

The proof step 4a98951 added failed on its first run with "generated/production_key.c was not compiled". The build log two steps above it disagrees:

[ 70%] Building C object CMakeFiles/eboot_core.dir/generated/production_key.c.obj

CMake names objects .obj under CMAKE_SYSTEM_NAME=Generic; the step searched for .o. So finding 1 is in fact answered — the TU is compiled into eboot_core and linked into eboot_firmware.elf on the cross toolchain — and the step meant to show it had a wrong glob. It failed closed on its own bug rather than passing vacuously, which is the failure you want from a proof step, but a red check that is wrong about why it is red helps nobody. 4d4e702 matches both namings; the find was run against a fixture tree with each extension and returns the object for both.

NOT RUN locally: the .obj naming itself — the host generator produces .o. The CI log line above is the evidence; the re-run on this head is the test.

Body range is now 31f44fb..4d4e702. Any approval at 4a98951 is dismissed; 4d4e702 is the head — could you re-approve. Holding pushes, red checks excepted.

[Kartikey1306]

Addendum to finding 3, a limitation the check has and the body should not imply it lacks. Measured (a sibling session's measurement, reproduced here at 4d4e702's checker):

Every single-hex-digit edit of the dev key — all 960 of them:

outcome	count	share
off the curve → refused	495	51.6%
on the curve, outside the prime-order subgroup → refused	413	43.0%
inside the prime-order subgroup → accepted	52	5.4%
genuinely low order	0	—

So about one mistyped digit in twenty produces a key that is a perfectly valid Ed25519 public key — just not the one whose private half anyone holds. No check on the key alone can catch that; it is indistinguishable from a real key by construction. The fleet it ships refuses every image, exactly as with an off-curve key, but with a green configure and a green release.

The check in this PR is therefore necessary and not sufficient. The sufficient control is comparing the secret against the key that was actually generated — ssh-keygen/openssl output, or the .pub beside the private key — before it is stored as EBLDR_PRODUCTION_KEY_HEX. The mixed-order message now says so ("check the secret against the key that was generated" is the sibling's wording; mine says "a mistyped hex digit usually lands here" — same instruction, and I'd take theirs as the clearer imperative in a follow-up). Worth a sentence in docs/key_lifecycle.md's provisioning steps; not pushing that now.

Two figures, for the record: the 413 mixed-order cases are the ones the previous wording called "low order", and the 0 in the last row is why no test ever challenged it.

[srpatcha]

Review — eBoot#122 "build: a Release build of a real board refuses to configure without a production trust anchor"

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

Verdict: Follow-up review of 8d09efbd..4d4e702a (2 commits). All three findings from the
previous review are resolved. The one that mattered — no pre-merge job cross-compiled the
production-key branch — I could close by actually running it this time: arm-none-eabi-gcc is
present on this host, which it was not for either earlier review, so the Cross-compile STM32F4
leg was executed end to end rather than reasoned about. It does what the commit claims. One new
finding, Low: the same .o/.obj naming difference that broke the proof step on its first run
is still present in tools/check_no_dev_anchor.py's suffix list, and it was fixed in only one of
the two places.

Previous findings — status

#	Previous finding	Status	Evidence
1	Medium (P2) — no pre-merge CI job compiled the production-key branch on any target; build/generated/production_key.c was first cross-compiled inside release.yml, on a tag	Resolved in 4a98951, glob corrected in 4d4e702	build.yml:56-62 now passes -DEBLDR_PRODUCTION_KEY=3d4017c3…660c -DCMAKE_BUILD_TYPE=Release — the RFC 8032 TEST 2 fixture I suggested — instead of -DEBLDR_ALLOW_DEV_KEY=ON. Ran the job's own commands against the real cross toolchain: configure rc 0, -- Trust anchor: production key from EBLDR_PRODUCTION_KEY; cmake --build build-arm --parallel 4 rc 0; the new proof step passes — build-arm/generated/production_key.c exists, build-arm/CMakeFiles/eboot_core.dir/generated/production_key.c.obj was built, and it carries the fixture key and not the dev key. ci.yml's ARM leg still compiles the dev-key half, so both branches of core/keystore.c are now cross-compiled before a tag.
2	Low (P3) — truncated Intel HEX record escaped as an IndexError traceback; p.suffix compared case-sensitively	Resolved in 4a98951	check_no_dev_anchor.py:67-73 adds the len(rec) < 5 guard and a record-length-vs-byte-count check the finding did not ask for; :137 catches IndexError and struct.error and now labels the exception type; :125-129 and :139 case-fold. struct is imported at :31, so the new except clause resolves. Pinned by test_a_truncated_hex_record_is_reported_not_raised and test_suffix_match_is_case_insensitive.
3	Low (P3) — "a point of low order" reported for every point outside the prime-order subgroup, wrong for the case an operator actually hits	Resolved in 4a98951	check_production_key.py:117-129 splits on [8]P == identity, so it is computed rather than a second blocklist. Ran the checker directly: 03·00³¹ → "on the curve but not in the prime-order subgroup -- a mistyped hex digit usually lands here", rc 1; ec·ff³⁰·7f → "a point of low order (order 2, 4 or 8); every signature would verify against it", rc 1; TEST 2 → accepted, rc 0. Two mixed-order vectors and a class-distinction test added.

The author's addendum on the thread — that ~5.4% of single-digit typos land inside the
prime-order subgroup and no check on the key alone can catch them, so the sufficient control is
comparing the secret against the generated .pub — is correct, is already said, and I am not
restating it as a finding. It should land in docs/key_lifecycle.md as they propose.

Findings

#	Severity	File:line	Finding	Recommended fix
1	Low (P3)	tools/check_no_dev_anchor.py:41	RAW_SUFFIXES lists .o but not .obj, so on every cross target the entry that exists to scan object files scans nothing. 4d4e702's commit message states the rule — "CMake names objects .obj, not .o, when CMAKE_SYSTEM_NAME=Generic" — and applies it to the proof step's find, but not to the scanner twenty lines away. Measured on the real cross build: every object under build-arm/ is *.c.obj, and the scanner's own summary line prints suffixes searched: .a .bin .efi .elf .hex .o .uf2. I checked whether this is exploitable rather than assuming: built the ARM stm32f4 tree with -DEBLDR_ALLOW_DEV_KEY=ON and ran the scanner over it — it still exits 1, flagging ebldr_stage0.bin, ebldr_stage0.elf and libeboot_core.a. So coverage holds today, but only through the archive and the linked images; the object-level check contributes nothing on the targets that ship. release.yml scans the whole build tree, and its esp32/esp32c3 legs configure through an ESP-IDF toolchain file that also sets CMAKE_SYSTEM_NAME=Generic, so those jobs are in the same position.	Add ".obj" to RAW_SUFFIXES at :41. test_the_scanner_covers_every_suffix_the_workflow_ships asserts shipped ⊆ SUFFIXES in one direction only, so widening the set does not break it — I checked the assertion before proposing this. Worth one line in the comment at :38-40 recording why both spellings are listed, since that is the second time this naming has cost a red check.

Correction to the thread, not a finding. 4d4e702's comment says the generated TU is "linked
into eboot_firmware.elf on the cross toolchain". It is not. arm-none-eabi-nm on the
production-key build finds ebldr_production_key, eos_keystore_init and
eos_keystore_get_active_key in ebldr_stage0.elf, and no symbols at all in
eboot_firmware.elf, which is text 0 data 0 bss 0 and whose .bin is 0 bytes. The claim the
finding actually needed — that the generated TU compiles and links into a real cross-built image —
is true via ebldr_stage0.elf, so the fix stands. The empty eboot_firmware is pre-existing on
origin/master and is what #138 in this same batch addresses; I am not charging it to this PR.

Verification performed for this review

Detached worktree at .ai/autoreview/state/scratch/eBoot-122 on 4d4e702a. The user's eBoot
checkout is on fix/ed25519-low-order-keys; it was clean before and is clean after, and was not
touched. Nothing was committed or pushed. prior-reviews.txt could not produce the incremental
diff (8d09efbd..4d4e702a not present locally), so the PR head was fetched into
refs/autoreview/scratch122 read-only.

Check	Result
cmake -B build/host -DEBLDR_BUILD_TESTS=ON -DCMAKE_BUILD_TYPE=Debug → cmake --build --parallel 4	PASS
ctest --test-dir build/host --output-on-failure --no-tests=error	PASS — 32/32, 11.20s
EOS_REQUIRE_SIGNING_TESTS=1 pytest tests/ -q	PASS — 131 passed, 9.17s (126 at the previously reviewed head, +5 from the two new test modules)
ARM cross-compile, production key — the job 4a98951 rewrote, run verbatim	configure rc 0, Trust anchor: production key from EBLDR_PRODUCTION_KEY; build rc 0, ebldr_stage0.elf linked
The new proof step, run verbatim against that tree	PASS — generated/production_key.c present; object found as …/generated/production_key.c.obj; fixture in o true, dev in o false. The .obj half of 4d4e702's find is load-bearing: with only the .o pattern the step fails, which is the bug that commit fixes, reproduced.
Anchor bytes in the linked cross images	ebldr_stage0.elf/.bin and libeboot_core.a carry the fixture; none carries the dev key
ARM cross-compile, -DEBLDR_ALLOW_DEV_KEY=ON (negative control for the scanner)	scanner rc 1, hits on ebldr_stage0.bin, ebldr_stage0.elf, libeboot_core.a. Zero .obj files inspected in either direction — finding 1.
tools/check_production_key.py direct vectors	order-2 → "low order, every signature would verify"; order-8L (03·00³¹) → "not in the prime-order subgroup"; TEST 2 → accepted. Three distinct messages, rc 1/1/0.
import struct present for the new except struct.error	CONFIRMED, :31
test_the_scanner_covers_every_suffix_the_workflow_ships direction	Asserts shipped - SUFFIXES == {}; adding .obj cannot break it

Architecture conformance

Conforms; re-checked against the two new commits rather than carried over. §21: eBoot is Tier 1
Foundation and every touched file — .github/workflows/build.yml, tools/, tests/unit/ — is
inside the owning repo; §21.1 is not engaged. §5.1 dependency direction is untouched: both changed
tools are build-time/test-time only and are linked into no image, and the generated TU remains a
leaf compiled into eboot_core. §5.1's "eBoot keeps the trusted computing base minimal and
auditable" is what 4a98951 serves directly — a trust anchor whose production branch had never
been compiled by any pre-merge job was not auditable, and it is now built and its contents asserted
on every PR. §14.1's "do not invent primitives" still holds: check_production_key.py re-applies
the shipped verifier's acceptance rule, it does not implement verification. §28's evidence policy is
honoured — every claim in the author's last four comments that I re-ran reproduced, including the
.obj one they explicitly marked NOT RUN.

Proposed changes

Not blocking. One line:

  tools/check_no_dev_anchor.py:41   RAW_SUFFIXES = {".elf", ".bin", ".a", ".o", ".obj", ".efi"}
                                    + a word in the comment above it saying why both      (finding 1)

Already agreed on the thread, worth not losing:

  docs/key_lifecycle.md             the ~5.4% of typos that land inside the prime-order
                                    subgroup are uncatchable from the key alone; check the
                                    secret against the generated .pub before storing it

Merge order is unchanged: #115 → #116 → #122. From this side the P1s that were the reason to
hold this branch have been closed and re-verified for two heads running, and the P2 that replaced
them is closed with the cross-compile actually executed rather than inferred.

No fix PR opened. tools/check_no_dev_anchor.py exists only on this branch — it is not on
origin/master — so a branch cut from the default branch, which is what fix-start.sh produces,
would have nothing to patch. This belongs to the author, and it is one line.

Not checked

• release.yml was not executed — it needs a tag. Statements about it come from reading the
  YAML and from running its scan step's script verbatim against local builds. The esp32/esp32c3
  claim in finding 1 is Inferred: I read that those legs pass an ESP-IDF toolchain file and
  know that file sets CMAKE_SYSTEM_NAME, but there is no Xtensa/RISC-V IDF toolchain on this host,
  so I did not observe .obj objects in those trees. The STM32F4 observation is Verified.
• esp32, esp32c3, rpi4, riscv64_virt, x86_64_efi cross builds — NOT RUN. Only stm32f4 via
  arm-none-eabi-gcc was built. x86_64_efi was built by the two earlier reviews, not by this one.
• CodeQL, cppcheck/clang-tidy, fuzz — NOT RUN locally. CI green on this head; logs not read.
• ASan/UBSan and Valgrind — NOT RUN.
• The UF2 decoder was exercised only through the PR's own fixtures. No real vendor-produced
  .uf2 here; "decodes what a real toolchain emits" remains Inferred for that format. The Intel HEX
  decoder was exercised against a real .hex only indirectly, through the tests' synthetic encoder.
• No published artifact was downloaded and inspected. The release-path reasoning is from source.
• docs/key_lifecycle.md rotation/HSM/custody content describes an operational process I cannot
  verify from the repository. Not re-read this round; unchanged since the last review.

---

Automated architecture review of 4d4e702a19dd — 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]

Taken, one push. Head is 1cd1265; numbers below measured there.

Finding 1 (Low) — .obj in RAW_SUFFIXES. Yes, and it is the same lesson twice: 4d4e702 fixed the proof step for .obj and left the scanner's own list on .o. Reproduced first — a .obj holding the dev key under build/ was reported clean. Added .obj with the reason in the comment above the set, and a test that puts the key in both a .o and a .obj and requires both found. Negative control: scanner at 4d4e702, test kept → 1 of 12 fails.

The docs line from the thread. docs/key_lifecycle.md gains a provisioning row: of 960 single-digit typos, 495 off-curve and 413 mixed-order are refused, but 52 land on a different valid key inside the subgroup and no check on the key alone can tell. The only control is comparing the secret to the generated .pub before it enters the secret store — and the mixed-order refusal message now ends with that instruction (the sibling session's wording, which is the clearer imperative).

pytest tests/   132 passed
ctest           100% tests passed, 0 tests failed out of 32

Merge order unchanged: #115 → #116 → #122. Range now 31f44fb..1cd1265. Any approval at 4d4e702 is dismissed by this push; 1cd1265 is the head — could you re-approve. Holding pushes, red checks excepted.

[srpatcha]

Review — eBoot#122 "build: a Release build of a real board refuses to configure without a production trust anchor"

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

Verdict: Follow-up review. The one finding from the review of 4d4e702a is resolved, and
I closed it by running the negative control the commit message claims: with .obj removed from
RAW_SUFFIXES the new test is the 1 of 12 that fails, exactly as stated. The eleven earlier
commits are patch-identical after the base-first re-stack — only a50df28 is new. One new
finding, Low, in the documentation row that same commit adds: I reproduced its typo statistic and
it is right for the development key, but the row presents that one key's count as the general
rate, and the general rate is worse.

Previous finding — status

#	Previous finding	Status	Evidence
1	Low (P3) — RAW_SUFFIXES listed .o and not .obj, so on every cross target the entry that exists to scan object files scanned nothing	Resolved in a50df28	tools/check_no_dev_anchor.py:44 is now {".elf", ".bin", ".a", ".o", ".obj", ".efi"}, with the reason recorded in the comment at :41-43 — which is the second half of what the finding asked for. Pinned by test_cross_target_object_files_are_scanned, which writes the dev key into both a keystore.c.o and a keystore.c.obj and requires scan() to return both, so it tests the scan and not just set membership. Negative control run: reverted RAW_SUFFIXES to the 4d4e702 spelling → test_cross_target_object_files_are_scanned FAILS, 1 failed / 11 passed in that module; file restored, git diff --stat clean.

The docs line agreed on the thread also landed, in docs/key_lifecycle.md:345 — see finding 1
below, which is about that line's wording, not its substance.

Findings

#	Severity	File:line	Finding	Recommended fix
1	Low (P3)	docs/key_lifecycle.md:345	A measurement taken on one specific key is stated as the general rate, and the general rate is worse than the number given. The new provisioning row says "of all single-hex-digit typos of a key, about one in twenty (52 of 960, measured) lands on a different valid key inside the subgroup". I recomputed the classification independently (decompress each of the 64×15 = 960 single-digit mutations, reject non-canonical y ≥ p and non-residues, then test [L]P == identity). 52 / 960 reproduces exactly — for d75a9801…, the RFC 8032 TEST 1 development key, together with the 495 and 413 in the commit message. For the RFC TEST 2 key that this PR uses as the production fixture, the same computation gives 64 / 960. The figure is key-dependent, and its expected value is not one in twenty: a mutated encoding decodes to a curve point with probability ≈ ½ and lands in the prime-order subgroup with probability ⅛, so the rate is ≈ 1/16 ≈ 60/960 for any key, by construction rather than by measurement. So the row understates the residual risk by about a quarter, and attributes a development-key count to the production key an operator will actually be typing. The substance of the row — that no check on the key alone can catch this case, and that comparing the secret against the generated .pub is the only control — is correct and is the part that matters.	One sentence: "about one in sixteen — a mutated encoding is a curve point with probability ½ and in the prime-order subgroup with probability ⅛; measured counts for two specific keys were 52/960 and 64/960." That makes the claim key-independent and derivable, which is what §28.1's benchmark policy wants of a number in a security document.

Nothing else new. The other eleven commits carry over from the previous three reviews with all
findings already closed there, and I confirmed they are unchanged rather than re-reading them.

Verification performed for this review

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

Check	Result
What is actually new since 4d4e702a?	One commit. git patch-id --stable over this PR's own commits at both heads: the eleven from 445476b…4d4e702 are identical to f8afc12…e5dbbb4; a50df28 is the only addition. The base-first re-stack the PR body describes (#115 a276016→e152d8e, #116 31f44fb→ef5b70d) is confirmed by the same method on those branches.
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 — 32/32, 11.35s
EOS_REQUIRE_SIGNING_TESTS=1 pytest tests/ -q	PASS — 134 passed, 9.96s. The commit message says 132; the two extra are tests/unit/test_sign_image.py cases that arrived on the new base from #117, not from this branch.
Negative control on the .obj fix — reverted RAW_SUFFIXES to {".elf",".bin",".a",".o",".efi"}, re-ran the module	test_cross_target_object_files_are_scanned FAILS, 1 failed / 11 passed. Matches the commit message's "1 of 12 fails (the .obj case)". File restored; git diff --stat clean afterwards.
The 52 of 960 claim, recomputed from scratch	Reproduces exactly for the development key — 495 off-curve / 413 outside the prime-order subgroup / 52 valid-in-subgroup, summing to 960. Independent implementation: edwards25519 decompression over p = 2²⁵⁵−19, d = −121665/121666, canonicality check y < p, then [L]P == identity with L = 2²⁵² + 27742317777372353535851937790883648493. For the RFC TEST 2 production fixture the same code gives 462 / 434 / 64. Both totals are 960 and no mutation produced a non-canonical encoding. This is the evidence for finding 1.
tools/check_production_key.py messages and exit codes at this head	Mixed-order 03·00³¹ → "on the curve but not in the prime-order subgroup … Check the secret against the key that was generated", rc 1; dev key → refused, rc 1; RFC TEST 2 → "a point in the prime-order subgroup of edwards25519, and not the development key", rc 0. The appended instruction from a50df28 is present and reachable, and the checker still fails closed.
Does a50df28 weaken anything?	No. Four files, +23/−2: one entry added to a set, one comment, one test added, one docs row added, one error message extended. No test disabled, no assertion removed, no lint loosened, no permission widened, no glob narrowed. SUFFIXES only grows, and test_the_scanner_covers_every_suffix_the_workflow_ships asserts shipped ⊆ SUFFIXES in one direction, so widening cannot mask a gap.

Architecture conformance

Conforms; re-checked against the one new commit rather than carried over. §21: eBoot is Tier 1
Foundation and all four touched files — docs/, tests/unit/, tools/ — are inside the owning
repo; §21.1 is not engaged. §5.1 dependency direction is untouched: both changed tools are
build-time/test-time only and are linked into no image, and nothing in this commit adds an
#include, link line or manifest entry of any kind. §5.1's "eBoot keeps the trusted computing
base minimal and auditable" is what the commit serves — an artifact scan that silently skipped
the object files on every cross target was auditing less than it reported. §14.1's "do not invent
primitives" still holds: check_production_key.py re-applies the shipped verifier's acceptance
rule rather than implementing verification. §28.1's benchmark policy is the clause finding 1 sits
under — a number in a security document needs its measurement definition stated, and this one's
definition is narrower than its wording.

No architecture proposal appended. The master-design gap this PR series exposes is already
filed: "A development trust anchor must be structurally unable to reach a release artifact"
(2026-09-13) in .ai/autoreview/proposals/2026-09.md. Finding 1 is a repository documentation
defect, not a gap in the design.

Proposed changes

Not blocking. One sentence:

  docs/key_lifecycle.md:345   "about one in twenty (52 of 960, measured)"
                              -> "about one in sixteen (1/2 on-curve x 1/8 in-subgroup);
                                 measured 52/960 for the development key and 64/960 for
                                 the RFC TEST 2 fixture"                        (finding 1)

Merge order unchanged: #115 -> #116 -> #122, as one batch.
Do not tag a release from a master that has #116 without this PR.

No fix PR opened. Unchanged reason from the previous review plus one more: docs/key_lifecycle.md
in its current form exists only on this branch, so a branch cut from the default branch — which is
what fix-start.sh produces — would have nothing to patch. It is also a one-sentence documentation
edit, which the brief's "small and provable by running something" bar does not cover.

Not checked

• release.yml was not executed — it needs a tag. Statements about it come from reading the
  YAML and from the PR's own workflow-parsing tests, not from a run.
• Cross-compile — NOT RUN this round. The STM32F4 production-key leg and the proof step were
  executed verbatim at 4d4e702a and passed; those eleven commits are patch-identical here, so
  that result carries, but it is Inferred from patch-id and not re-observed at this SHA. The
  esp32, esp32c3, rpi4, riscv64_virt and x86_64_efi legs have never been built locally.
• The .obj fix was verified through the scanner's own scan() on synthetic files, not by
  re-running a real cross build and watching it inspect *.c.obj. The end-to-end observation at
  4d4e702a was that zero .obj files were inspected; I did not re-do the cross build to observe
  a non-zero count now.
• CodeQL, cppcheck/clang-tidy, fuzz-harness — NOT RUN locally. CI green on this head; job logs
  not read.
• ASan/UBSan and Valgrind — NOT RUN.
• The typo statistic was computed for two keys only. 52/960 and 64/960 are Verified; "≈1/16 for
  any key" is a derivation from the ½ × ⅛ structure of the encoding, not an exhaustive measurement.
• No published artifact was downloaded and inspected. The release-path reasoning is from source.
• docs/key_lifecycle.md's rotation, HSM and custody content describes an operational process I
  cannot verify from the repository. Unchanged since the last review and not re-read.

---

Automated architecture review of a50df2809bf0 — 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]

Taken — one sentence, one push. Head 18fab20.

You were right on both counts, and a third key makes it plainer. Re-measured at this head with the branch's own checker over all 960 single-digit mutations: dev key 52/960, RFC TEST 2 64/960, RFC TEST 3 75/960 — the spread around 60 that 1/2 × 1/8 = 1/16 predicts. The row now gives the derivable rate and cites all three as measurements of specific keys, so the number is reproducible without the key it was taken on. Your restated-finding-1 (.obj) is confirmed resolved at a50df28 and untouched here.

Any approval at a50df28 is dismissed; 18fab20 is the head — could you re-approve. Holding pushes.