fix(stage0): refuse a stage-1 image the flash could not read

[Kartikey1306]

Stacked on #115 (master does not build without it). Review e152d8e..e018474 for this change alone: three commits -- e018474 closes the one carried-forward guard finding (a call as a one-line if/while body counted as examined; the call must sit inside the condition's parentheses, two more bypass probes in the test) (restacked once, onto #115's e152d8e; a276016..949775b was the same content on the previous base). 1de05d6 (was 4b42626) is the fix -- stage0/jump_stage1.c (+11), tests/unit/test_stage0_hal_results.py (new), CHANGELOG.md. d68d5d1 (was 949775b) answers the review's findings 2, 3 and 4 (stage0/jump_stage1.c, include/eos_types.h, the guard, CHANGELOG.md). Finding 1 (Critical, pre-existing: the stage-1 image was empty) is issue #136 and PR #138, stacked on this head.

Problem (eBoot #128)

The stage-1 hash loop in stage0/jump_stage1.c discarded eos_hal_flash_read()'s result:

eos_hal_flash_read(stage1_addr + off, buf, chunk);
eos_sha256_update(&sha_ctx, buf, chunk);

A read that fails leaves buf holding the previous chunk — or whatever the stack held on the first iteration — and that is hashed as if it were stage-1. The mismatch that follows sends the device to recovery, so the outcome was fail-closed by accident, and it was logged as 0xBAD1 (hash mismatch), which is not what happened. core/crypto_boot.c's eos_crypto_verify_image() does the same job and refuses a failed read; this is the class #38 fixed for eos_crc32().

Change

The read is tested. On failure stage-0 logs EOS_LOG_BOOT_FAIL with its own reason (0xBAD2), enters recovery, and returns so the loop can never fall through to the jump. The double comparison that follows (fault-injection hardening) is untouched.

Test

stage0/ is only compiled by a cross build, so the test is a source-level guard in the style of test_stage0_reset_entry.py:

• every call to a HAL read/write (eos_hal_flash_read/write/erase, eos_hal_otp_read/write, eos_hal_monotonic_read) in stage0/*.c must assign or test its result;
• the stage-1 loop specifically must test the read, enter recovery on failure with 0xBAD2, and return.

Verified: both tests fail against the unfixed file; 5 passed on this head; pytest tests/ 85 passed. cc -fsyntax-only -DEBLDR_VERIFY_STAGE1 -Iinclude stage0/jump_stage1.c is clean; the Cortex-M4 cross-compile was also run locally with build.yml's flags (Arm GNU Toolchain 14.2.Rel1): ebldr_stage0.bin 12,904 bytes.

Review follow-up (949775b)

Finding	What changed
1 (P0, pre-existing) -- eboot_firmware.bin is 0 bytes and stage-0 verifies it	Not in this PR, as the review asks. Issue #136, PR #138 stacked on this head: stage1/reset_entry.c + ENTRY/KEEP in the stage-1 linker script (stm32f4 stage-1 links to 13,416 bytes), the embed tool refuses an empty or sub-floor image, stage-0 refuses stage1_expected_size == 0.
2 (P1) -- the mismatch path fell through to the jump	return; after eos_recovery_enter() in the mismatch block, so IMAGE_VALID is written only when the hash matched; the recovery-trigger path eight lines earlier had the same shape and returns too. test_every_recovery_entry_in_stage0_is_followed_by_return asserts every eos_recovery_enter() in ebldr_stage0_main() is followed by return (a closing brace counts only when it closes the function) and that the mismatch block returns and carries no IMAGE_VALID. Negative control against 4b42626: fails on the mismatch call.
3 (P2) -- the guard's false negatives (two, then a third: a one-line if/while body)	Preprocessor lines are stripped with the comments (closes the #ifdef probe); a result counts as examined only when the call sits in an if/while condition or a return, or is assigned to a name a later if/while/return in the same block reads (closes int rd = …; (void)rd;). test_the_guard_rejects_an_assigned_but_untested_result runs both probes against the guard and asserts they are refused, and that the two legitimate shapes are accepted.
4 (P3) -- 0xBAD1/0xBAD2 bare	EBLDR_FAIL_STAGE1_HASH 0xBAD1 and EBLDR_FAIL_STAGE1_READ 0xBAD2 in include/eos_types.h (after the size constants, so the block does not collide with #127's EOS_LOG_AUTH_* lines), used by name in jump_stage1.c. #138 adds EBLDR_FAIL_STAGE1_NO_IMAGE 0xBAD3 beside them.

build_sim/ being committed is noted; not touched here.

Closing issue

Fixes #128

[codecov-commenter]

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

Codecov Report

❌ Patch coverage is 90.76923% with 12 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
tests/unit/test_stage0_hal_results.py	90.00%	5 Missing and 5 partials ⚠️
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#129 "fix(stage0): refuse a stage-1 image the flash could not read"

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

Verdict: The change itself is correct, minimal and well-tested — I reverted it and both new
tests went red, and I cross-compiled it for Cortex-M4, which is stronger than the syntax-only
check the body claims. Reviewed a276016..4b42626 (one commit) as the body asks. But going after
this defect took me into the surrounding block, and the stage-1 integrity check this PR hardens
verifies nothing at all on every build where it exists. I reproduced that three ways. It is
pre-existing and not this PR's doing; it is also the reason the loop this PR fixes cannot execute
on a real build, so it belongs in this review rather than a later one.

Findings

#	Severity	File:line	Finding	Recommended fix
1	Critical (P0) — pre-existing, not introduced by this PR	tools/embed_stage1_hash.py:96-98; stage0/jump_stage1.c:76-108; boards/stm32f4/stm32f4_stage1.ld; CMakeLists.txt:387-399	Stage-0 reports IMAGE_VALID and jumps to stage-1 having hashed zero bytes. eboot_firmware.bin is 0 bytes on every board build that has a stage-1 linker script. stage1/main.c exports eboot_main(), not main, and the target links with -nostartfiles against a stage-1 linker script with no ENTRY() and no KEEP(*(.isr_vector)) (the stage-0 script has one at :21), so the linker has nothing to anchor and arm-none-eabi-objdump -h reports .text 00000000. embed_stage1_hash.py has no minimum-size check — size = os.path.getsize(input_path) at :96 is used verbatim — so it emits stage1_expected_size = 0u and stage1_expected_hash = e3b0c442…b855, the SHA-256 of the empty string, and prints it as a normal build line. On the device: while (off < 0) at :76 runs zero iterations (so the read check this PR adds never executes), computed equals stage1_expected_hash trivially, match1 == match2 == 0, EOS_LOG_IMAGE_VALID is appended at :108, and ops->jump(stage1_addr) transfers control to whatever is in flash. The first link of the secure boot chain reports success without having run — the failure mode .ai/security.md names first — and does it while writing a positive attestation record, which that file also calls out separately. CMakeLists.txt:336-340 describes this mechanism as "the first link of the secure boot chain"; it currently measures nothing. CI is green over all of it.	Two independent fixes, and both are needed, because either alone leaves a hole: (a) embed_stage1_hash.py must refuse a zero-byte — and realistically an implausibly small — input, rather than hashing it: if size == 0: print("stage-1 image is empty; there is nothing to verify", file=sys.stderr); return 1. (b) the stage-1 link has to produce an image: add ENTRY(<stage-1 reset handler>) and KEEP(*(.isr_vector)) to boards/stm32f4/stm32f4_stage1.ld and boards/cortex_r5/cortex_r5_stage1.ld, matching what the stage-0 scripts already do. Belt and braces in the C as well: if (stage1_expected_size == 0) { log; recovery; return; } before the loop — a bootloader should not trust its own build to have been correct. Expect (a) to turn the cross-compile jobs red until (b) lands; that is the point, and it is why this needs a human to sequence rather than an autofix.
2	High (P1) — pre-existing	stage0/jump_stage1.c:104-108	The hash-mismatch path falls through to the jump. if (match1 || match2) { eos_boot_log_append(…, 0xBAD1); eos_recovery_enter(&bctl); } — no return. Control then reaches eos_boot_log_append(EOS_LOG_IMAGE_VALID, …) at :108 and ops->jump(stage1_addr) at :114. The only thing stopping a jump into an image that failed verification is that eos_recovery_enter() (core/recovery.c:449) is a while (1) with no break — an invariant contradicted by its own int return type and return EOS_OK;, and by this file's comment at :55 ("Does not return unless recovery instructs a reboot"). I traced it and it does not return today (the only exits are eos_hal_system_reset() in RCVR_CMD_RESET and recovery_handle_boot), so this is not reachable right now and I am not claiming otherwise. It is still the same defect this PR fixes eight lines above, for the same reason — and the PR's own new test asserts "return;" for the read path while leaving the mismatch path unpinned, so the asymmetry is now baked into the test suite.	return; after eos_recovery_enter(&bctl); at :106, and put eos_boot_log_append(EOS_LOG_IMAGE_VALID, …) in the else so a failed verify cannot write a positive record. Add the assertion to test_the_stage1_hash_loop_refuses_a_failed_read, or better, a sibling test over the whole #ifdef EBLDR_VERIFY_STAGE1 block: every eos_recovery_enter( in stage0/ is followed by return.
3	Medium (P2)	tests/unit/test_stage0_hal_results.py:35-48	The general guard has two demonstrated false negatives, and today it covers exactly one call site — the one the specific test already pins. I enumerated what it inspects: 1 statement, jump_stage1.c's fixed read. So its whole value is future-facing, which is where the holes bite. Both demonstrated by editing the file and running the module: (a) int rd = eos_hal_flash_read(…); (void)rd; → test_every_hal_read_and_write_in_stage0_examines_its_result passes, because _result_is_examined() accepts a bare = in the head; the docstring says "assigned, or tested", so the test matches its contract but not its name, and an assigned-then-ignored result is the same defect in a different shape. (b) placing the discarded call immediately after a #ifdef line → passes, because _result_is_examined() tests "if" in head and #ifdef supplies the substring. Both were caught only by test_the_stage1_hash_loop_refuses_a_failed_read, which is hard-coded to that one loop and does not generalise to the other four stage0/*.c files the guard claims to cover.	In _strip_comments(), also drop preprocessor lines (re.sub(r"(?m)^\s*#[^\n]*", " ", text)) — that closes (b) and is one line. For (a), match the assigned name and require it to appear again in a test before the enclosing block ends, or keep it simple and require the call to be inside a condition: `re.match(r"(if
4	Low (P3)	stage0/jump_stage1.c:86	0xBAD2 joins 0xBAD1 as a bare magic detail code defined in no header and listed in no document, so a field engineer reading a boot log gets event=BOOT_FAIL detail=0xBAD2 and no way to learn what it means. Same class as the 0x22 finding on #127 — include/eos_types.h:134-145 has an EOS_LOG_* namespace that stage0/ and core/recovery.c both bypass.	#define EBLDR_FAIL_STAGE1_HASH 0xBAD1 / EBLDR_FAIL_STAGE1_READ 0xBAD2 beside the EOS_LOG_* codes, used by name. Worth doing once across both PRs rather than twice.

Adjacent, not a finding on this PR: build_sim/ is committed — 81 tracked files including
build_sim/CMakeCache.txt, which carries absolute paths from whoever generated it. A fresh clone
that runs cmake -B build_sim inherits a stale cache pointing at another machine's directories.
Untouched by this PR; recorded here because this is where I hit it.

What this PR gets right

The fix is exactly the right size: test the read, log a distinct reason rather than reusing the
hash-mismatch code, enter recovery, and return so the loop cannot fall through — and that last
word is the one that matters, as finding 2 shows. Choosing a source-level guard because stage0/
is cross-only is the right call rather than an excuse; test_stage0_reset_entry.py set the
precedent. And the body's claim that the old behaviour was fail-closed by accident is accurate
and worth the sentence it got: a hash over stack garbage mismatching is luck, not a control.

Verification performed for this review

Detached worktree at .ai/autoreview/state/scratch/eBoot-129 on refs/pull/129/head. The user's
eBoot checkout was clean before and after and was not touched; nothing was committed or pushed.
refs/autoreview/pr129 was not present locally, so I fetched refs/pull/129/head read-only into a
scratch ref to isolate a276016..4b42626.

Check	Result
pytest tests/ -q	PASS — 83 passed, 1.69s — matches the body
pytest tests/unit/test_stage0_hal_results.py -q	PASS — 3 passed
Negative control — the fix reverted to the bare eos_hal_flash_read(…);	2 of 3 FAIL (test_every_hal_read_and_write_in_stage0_examines_its_result, test_the_stage1_hash_loop_refuses_a_failed_read). The tests catch the defect they claim to. Restored; git status clean.
Bypass probe A — int rd = …; (void)rd;	general guard PASSES (finding 3)
Bypass probe B — call placed after #ifdef	general guard PASSES (finding 3)
Call sites the general guard actually inspects	1 — enumerated by importing the module's own helpers
cc -fsyntax-only -DEBLDR_VERIFY_STAGE1 -Iinclude stage0/jump_stage1.c	PASS, rc 0 — the body's claim reproduces
Cortex-M4 cross-compile — build.yml's flags verbatim (-DCMAKE_SYSTEM_NAME=Generic -DCMAKE_C_COMPILER=arm-none-eabi-gcc -DEBLDR_BOARD=stm32f4 …)	PASS — configure rc 0, build rc 0, ebldr_stage0.elf links, ebldr_stage0.bin 12928 bytes. This is the check the body defers to CI; it passes.
Same, via -DCMAKE_TOOLCHAIN_FILE=toolchains/arm-none-eabi.cmake	PASS, rc 0
eboot_firmware.bin size, both stm32f4 builds	0 bytes, and stage1_hash.c generated with SHA-256: e3b0c442…b855 / stage1_expected_size = 0u — finding 1
cortex_r5 cross build (toolchains/arm-none-eabi-r5.cmake)	configure rc 0, build rc 0, eboot_firmware.bin 0 bytes, same empty-string hash. These are the only two boards with a *_stage1.ld, so the mechanism is vacuous on 100% of the builds where it is live.
arm-none-eabi-objdump -h build-arm/eboot_firmware.elf	.text 00000000, .data 00000000, .bss 00000000 — establishes the cause of finding 1
grep -n ENTRY boards/*/\*_stage1.ld	no matches on either board; stm32f4_stage0.ld:21 has KEEP(*(.isr_vector)) and the stage-1 script has neither
eos_recovery_enter() traced (core/recovery.c:449-525)	while (1) with no break; exits only via eos_hal_system_reset(). Confirms finding 2 is unreachable today and confirms why.
EBLDR_VERIFY_STAGE1 default	CMakeLists.txt:26 — ON. The block is live, not dormant.
0xBAD1 / 0xBAD2 uniqueness	no collision; both undefined anywhere as symbols

Architecture conformance

The PR conforms; the surrounding code does not. §21: eBoot is Tier 1 Foundation, all files are in
the owning repo, §21.1 is not engaged. §5.1 dependency direction holds — stage0/ calls into
hal/, core/ and include/ and nothing points up a tier. The clause this review turns on is
§5.1's "eBoot keeps the trusted computing base minimal and auditable" together with §8's boot
flow and §8.1's required concepts. Finding 1 is a direct violation of both readings: a measurement
step that is structurally incapable of measuring anything is not auditable, and a chain diagram
whose first link is a no-op is not the chain §8 describes. §28's status policy is the other half —
docs/secure_boot_chain.md and CMakeLists.txt:336 both describe stage-1 verification in the
present tense, which §28 permits only for Implemented ("code and functional tests"); on the
evidence above it is at best Planned. The narrower gap — that §8.1 enumerates signed manifests,
signed images, A/B slots, rollback protection and recovery but is silent on the stage-0 → stage-1
measurement and therefore silent on what a build must do when the stage-1 image is unavailable at
hash time — is the design gap that let this ship, and I have appended it as a proposal to
.ai/autoreview/proposals/2026-09.md.

Proposed changes

This PR, as it stands: mergeable on its own merits. The fix is correct and pinned.

Then, in priority order and NOT in this PR:

  P0  tools/embed_stage1_hash.py   refuse a zero-byte stage-1 image (exit 1)
      boards/{stm32f4,cortex_r5}/*_stage1.ld
                                   ENTRY() + KEEP(*(.isr_vector)), as the
                                   stage-0 scripts already have
      stage0/jump_stage1.c         refuse stage1_expected_size == 0 before
                                   the loop -- do not trust the build
      (a) will redden the cross jobs until (b) lands; sequence it deliberately

  P1  stage0/jump_stage1.c:106     return; after eos_recovery_enter(), and
                                   EOS_LOG_IMAGE_VALID only in the else
      tests/                       assert it, the way this PR asserts the
                                   read path

  P2  test_stage0_hal_results.py   strip preprocessor lines; require the
                                   result to be tested, not merely assigned

  P3  include/eos_types.h          symbolic names for 0xBAD1/0xBAD2

No fix PR opened, deliberately. Finding 1 is Critical, and the brief's autofix rule is
explicitly High only, small and provable. It is neither small nor safely provable from here:
making embed_stage1_hash.py refuse an empty image turns both cross-compile jobs red until the
linker scripts are repaired, and I cannot verify a repaired stage-1 image boots without hardware.
That sequencing is a human's call. Finding 2 is High and on origin/master, but adding a return
to a secure-boot failure path changes what a device does when verification fails, and "verify by
running something" is not available to me for that path. Finding 3 is a test-only change and would
qualify, but it is Medium. So: nothing opened, and I would rather say that than open something I
cannot stand behind.

Not checked

• No hardware, and no emulation of the boot path. Everything about device behaviour is read
  from source plus the cross-compiled artifacts. I did not run stage-0 on a target or in QEMU,
  so finding 1's consequence ("jumps having verified nothing") is Observed in the code and the
  generated stage1_hash.c, not demonstrated on a device.
• cortex_r5 was built but its .text was not disassembled — I inferred the same empty-image
  cause from the identical 0-byte output and the identical linker-script shape. Inferred, not
  Verified.
• The other ~40 board directories were not built. CMakeLists.txt:341 only embeds the hash when
  a *_stage1.ld exists, and only two boards have one, so I believe the blast radius is those two
  — but I have not built every board to prove no third path exists.
• release.yml's eight board builds were NOT run (they need a tag). Reading them, they set no
  toolchain file, so if(CMAKE_CROSSCOMPILING) is false and this whole block is skipped for them —
  which means the released firmware artifacts do not contain a stage-1 hash at all. I did not
  verify that by building them. Inferred.
• The #115 half of the 27-file diff was not reviewed here. This review covers
  a276016..4b42626 only.
• ASan/UBSan, Valgrind, fuzz, CodeQL, cppcheck/clang-tidy — NOT RUN locally. CI green on this
  head; logs not read.
• mergeStateStatus is BLOCKED — REVIEW_REQUIRED plus the #115 stack. I did not verify
  #115 is mergeable.

---

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

Review — eBoot#129 "fix(stage0): refuse a stage-1 image the flash could not read"

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

Verdict: Follow-up review of 4b42626f..949775b3 (one commit). Findings 2, 3 and 4 are
resolved, each with a negative control I re-ran rather than read. Finding 1 — the Critical — is
untouched here, correctly so, and I re-measured it at this head: eboot_firmware.bin is still
0 bytes and stage1_expected_size is still 0u, so the stage-1 measurement still verifies
nothing on both boards where it is live. #138 in this same batch is the PR that addresses it.
One new finding against the hardened guard, Medium: it now rejects an assigned-and-discarded result,
but a result discarded inside if (cond) hal_call(...); still passes, because the check only asks
whether the statement starts with if.

Previous findings — status

#	Previous finding	Status	Evidence
1	Critical (P0) — pre-existing — stage-0 reports IMAGE_VALID and jumps having hashed zero bytes; eboot_firmware.bin is 0 bytes on both boards with a stage-1 linker script	Open. Untouched by this PR, correctly.	Re-measured at 949775b3: cmake -DCMAKE_TOOLCHAIN_FILE=toolchains/arm-none-eabi.cmake -DEBLDR_BOARD=stm32f4 → configure rc 0, build rc 0, build-arm/eboot_firmware.bin 0 bytes, build-arm/stage1_hash.c:18 const uint32_t stage1_expected_size = 0u;, :6 SHA-256: e3b0c442…b855 — the empty string. grep -l ENTRY boards/*/*_stage1.ld → no matches on either stm32f4_stage1.ld or cortex_r5_stage1.ld; tools/embed_stage1_hash.py:96 still uses os.path.getsize() with no minimum-size check. Nothing about this has moved. It is out of scope for this PR and blocking this PR on it would be wrong; #138 is where it is being answered and I review that separately.
2	High (P1) — pre-existing — the hash-mismatch path falls through to the jump; only eos_recovery_enter()'s while (1) stops it	Resolved in 949775b	stage0/jump_stage1.c:112-113 adds return; after eos_recovery_enter(&bctl) in the mismatch branch, and :54-56 adds the same to the recovery-trigger call the finding did not ask about — the right instinct, same defect. The author chose return over moving EOS_LOG_IMAGE_VALID into an else; equivalent and less nesting. Negative control re-run: deleting that return; → test_every_recovery_entry_in_stage0_is_followed_by_return FAILS at :133. I also traced where the new return goes: stage0/reset_entry.c:77-80 calls ebldr_stage0_main() and follows it with while (1);, so returning hangs rather than running off the end — fail-closed, which is what a refused verification should do.
3	Medium (P2) — the general guard accepted int rd = read(…); (void)rd; and a call placed after a #ifdef	Resolved in 949775b	_strip_comments() now drops preprocessor lines (:33), closing the #ifdef-supplies-an-if bypass. _result_is_examined() (:60-83) requires an assigned name to be read by a later if/while/return in the same block, via the new _rest_of_enclosing_block(). Both of my demonstrated bypasses are now pinned as explicit cases in test_the_guard_rejects_an_assigned_but_untested_result, with two positive controls beside them — the right shape, since a guard that only tests its negatives can be broken into always-False. Re-ran the four cases directly against the module's helpers: (void)rd → False ✅, post-#ifdef → False ✅, rc then if (rc != EOS_OK) → True ✅, inline if (read(…) != EOS_OK) → True ✅.
4	Low (P3) — 0xBAD1/0xBAD2 bare magic detail codes named nowhere	Resolved in 949775b	include/eos_types.h:153-157 defines EBLDR_FAIL_STAGE1_HASH 0xBAD1 and EBLDR_FAIL_STAGE1_READ 0xBAD2 with a comment saying what they attach to; both used by name at stage0/jump_stage1.c:88,111; test_the_stage1_hash_loop_refuses_a_failed_read now asserts the name rather than the literal.

The build_sim/ observation recorded last time as adjacent, not a finding is unchanged and still
not charged to this PR.

Findings

#	Severity	File:line	Finding	Recommended fix
1	Medium (P2)	tests/unit/test_stage0_hal_results.py:74	A HAL result discarded inside a one-line if or while body still passes the guard. _result_is_examined() returns True as soon as re.match(r"(if|while)\s*\(", head) matches, which asks only whether the statement starts with if — not whether the call is inside the condition. Demonstrated against the module's own helpers at this head: if (need) eos_hal_flash_read(0,0,0); → examined=True; while (n--) eos_hal_flash_read(0,0,0); → examined=True. In both the return value goes nowhere, which is the exact defect the guard exists to catch and the exact defect this PR fixes eight lines away in jump_stage1.c. This is the third bypass of the same helper in two review rounds; the first two are now self-tested and this class is not, so it will be found the same way — by someone reading it — rather than by the suite. Not reachable in stage0/ today: I enumerated the call sites and none is written this way, so the guard is correct about the current tree and this is future-facing, same as findings 3's were.	The distinction the check actually wants is is the call inside the condition's parentheses, which is one line: re.match(r"(if|while)\s*\(", head) and head.count("(") > head.count(")"). I verified it discriminates on all six shapes: "if (" → True, "while (" → True, "if (eos_ok && " → True, "if (need) " → False, "while (n--) " → False, "return " → False (still caught by the return branch above it). Add both negative cases to test_the_guard_rejects_an_assigned_but_untested_result, beside the two already there.

Verification performed for this review

Detached worktree at .ai/autoreview/state/scratch/eBoot-129 on 949775b3. 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. 4b42626f..949775b3 was not present locally, so the PR
head was fetched into refs/autoreview/scratch129 read-only. Every probe was reverted and
git status --short confirmed empty before the next.

Check	Result
pytest tests/ -q	PASS — 85 passed, 1.98s (83 at the previously reviewed head)
pytest tests/unit/test_stage0_hal_results.py -q	PASS — 5 passed (3 before)
Negative control — return; removed from the mismatch branch	test_every_recovery_entry_in_stage0_is_followed_by_return FAILS at :133. Restored; tree clean.
Finding 3's two bypasses, re-run against the new helpers	(void)rd → False ✅ · post-#ifdef → False ✅ · assigned-then-tested → True ✅ · inline condition → True ✅
New bypass probe (finding 1)	if (need) read(…) → True ❌ · while (n--) read(…) → True ❌ · bare read(…) → False ✅ · return read(…) → True ✅
Proposed fix discriminates	"if ("/"while ("/"if (eos_ok && " → True; "if (need) "/"while (n--) " → False; "return " → False
Cortex-M4 cross-compile via toolchains/arm-none-eabi.cmake, EBLDR_BOARD=stm32f4	PASS — configure rc 0, build rc 0, ebldr_stage0.bin 12904 bytes. The changed stage0/jump_stage1.c compiles on the target toolchain, not just the host.
Finding 1 re-measured at this head	eboot_firmware.bin 0 bytes; stage1_hash.c:18 stage1_expected_size = 0u; :6 hash e3b0c442…b855; no ENTRY() in either *_stage1.ld; no size guard in embed_stage1_hash.py:96. Unchanged.
Where the new return lands	stage0/reset_entry.c:77-80 — ebldr_stage0_main() is followed by while (1);. Fail-closed.
stage0/ call sites currently written as a one-line if body	none — finding 1 is not live in this tree

Architecture conformance

The PR conforms; the surrounding code still does not, for the reason recorded last time. §21: eBoot
is Tier 1 Foundation, every file is in the owning repo, §21.1 not engaged. §5.1 dependency direction
holds — stage0/ calls down into hal/, core/ and include/, and moving 0xBAD1/0xBAD2 into
include/eos_types.h moves a private literal into the contract header, which is the correct
direction. §5.1's "eBoot keeps the trusted computing base minimal and auditable" is what the new
commit serves directly: a verification failure that logged a negative record and then wrote a
positive one and jumped anyway was not auditable, and it now stops. §8's boot flow — Verify
Manifest → Verify Image → … → Load EoS → Transfer Control — is honoured on this path for the first
time: control no longer reaches Transfer Control from a failed verify. §8.1's silence on the
stage-0 → stage-1 measurement is still the design gap behind finding 1, and the proposal appended
during the previous review stands unchanged; I have not duplicated it.

Proposed changes

In this PR (one line and two assertions):
  test_stage0_hal_results.py:74   and head.count("(") > head.count(")")
  test_stage0_hal_results.py      `if (need) read(...)` and `while (n--) read(...)`
                                  as negative cases beside the two already there

Not this PR, unchanged in priority from the previous review:
  P0  tools/embed_stage1_hash.py      refuse a zero-byte stage-1 image
      boards/{stm32f4,cortex_r5}/*_stage1.ld   ENTRY() + KEEP(*(.isr_vector))
      stage0/jump_stage1.c            refuse stage1_expected_size == 0 before the loop
      -> tracked by #138; sequence (a) and (b) together or the cross jobs go red

This PR remains blocked behind #115 (a276016 and below belong to that PR; 24 of the 27 files in
the bundle diff are its). mergeStateStatus was BLOCKED at the previous review and I did not
re-check it here.

No fix PR opened. tests/unit/test_stage0_hal_results.py exists only on this branch — not on
origin/master — so a branch cut from the default branch, which is what fix-start.sh produces,
would have nothing to patch. Finding 1's fixes remain neither small nor provable from here for the
reasons given last time, and they now belong to #138.

Not checked

• No hardware, and no emulation of the boot path. Everything about device behaviour is read from
  source plus cross-compiled artifacts. Stage-0 was not run on a target or in QEMU, so finding
  1's consequence remains Observed in the code and in the generated stage1_hash.c, not
  demonstrated on a device.
• cortex_r5 was not rebuilt this round. The stm32f4 measurement above is Verified; that
  cortex_r5 is in the same state is Inferred from its identically shaped linker script.
• The other board directories were not built. Only two have a *_stage1.ld, so I believe the
  blast radius is those two, but I have not proved no third path exists.
• release.yml's board builds were NOT run — they need a tag. The previous review's reading,
  that they set no toolchain file and therefore skip this block entirely, is unchanged and still
  Inferred.
• The #115 half of the 27-file diff was not reviewed here. This review covers
  4b42626f..949775b3 only.
• ASan/UBSan, Valgrind, fuzz, CodeQL, cppcheck/clang-tidy — NOT RUN locally. CI green on this
  head; logs not read.
• No author comment accompanies 949775b — only codecov has posted since the last review. The
  three resolutions above are read from the diff and re-verified here, not taken from a claim.

---

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

Thanks — this is the most useful review this stack has had. Findings 2, 3 and 4 are in 949775b on this branch (range now a276016..949775b; the body has a "Review follow-up" table). Finding 1 is done as the separate PR you asked for: issue #136, PR #138, stacked on this head.

Finding 1 — the stage-1 image was empty. Reproduced exactly as you describe, with build.yml's flags and with toolchains/arm-none-eabi.cmake (Arm GNU Toolchain 14.2.Rel1): eboot_firmware.bin 0 bytes, stage1_hash.c carrying e3b0c442…b855. #138 does all three of the things you listed, because each alone still leaves a hole: stage1/reset_entry.c owns the vector table and Reset_Handler (into eboot_main()), stm32f4_stage1.ld gets ENTRY(Reset_Handler) and a kept .isr_vector placed first, embed_stage1_hash.py exits 1 for an empty image whatever --min-size is and for anything under a 4096-byte floor, and jump_stage1.c refuses stage1_expected_size == 0 before the loop with EBLDR_FAIL_STAGE1_NO_IMAGE. Measured after: stm32f4 stage-1 links to 13,416 bytes with eos_ed25519_verify, the SHA cores, the keystore, rollback and recovery in it; first two words 0x20020000 / 0x08004211 (stack top, Reset_Handler with the Thumb bit — what stm32f4_jump() loads); the embedded digest equals shasum -a 256 of the image. One more thing turned up on the way and is fixed there too: stage-0's own table stopped at UsageFault while board_early_init() enables the SysTick interrupt (vector 15), so the first tick fetched a handler address from the code after the table; both tables now run through SysTick. cortex_r5 is filed as #137 — its stage-0 script keeps .vectors, which nothing emits, so its stage-0 was 0 bytes as well; no PR-gating workflow builds it.

Finding 2 — the mismatch path fell through. return; after eos_recovery_enter() in the mismatch block, so IMAGE_VALID is written only when the hash matched. The recovery-trigger path eight lines earlier (if (ebldr_recovery_triggered(&bctl)) { eos_recovery_enter(&bctl); }) had the same shape and returns too. The sibling test you suggested is test_every_recovery_entry_in_stage0_is_followed_by_return: every eos_recovery_enter() in ebldr_stage0_main() must be followed by return — a closing brace counts only when it closes the function — and the mismatch block must return and carry no IMAGE_VALID. Negative control against 4b42626: it fails on the mismatch call.

Finding 3 — the guard's false negatives. Both probes reproduced against the old guard ((void)rd → examined; after #ifdef → examined). Preprocessor lines are now stripped with the comments, and a result counts as examined only when the call sits in an if/while condition or a return, or is assigned to a name that a later if/while/return in the same block reads (the block found by brace matching from the statement's end). test_the_guard_rejects_an_assigned_but_untested_result runs your two probes through the guard's own helpers and asserts both are refused, and that int rc = …; if (rc != EOS_OK) and the inline if (eos_hal_flash_read(…) != EOS_OK) are accepted. You are right that today it inspects one statement; it is future-facing, and now it does not lie about the shapes it accepts.

Finding 4 — bare detail codes. EBLDR_FAIL_STAGE1_HASH 0xBAD1 / EBLDR_FAIL_STAGE1_READ 0xBAD2 in include/eos_types.h, used by name; #138 adds EBLDR_FAIL_STAGE1_NO_IMAGE 0xBAD3 beside them. I put the block after the size constants rather than directly under EOS_LOG_BOOT_FAIL, because #127's EOS_LOG_AUTH_* lines land there and the two stacks would otherwise conflict at merge.

pytest tests/ on 949775b: 85 passed (83 + 2). build_sim/ being committed is noted and left alone here.

[srpatcha]

Review — eBoot#129 "fix(stage0): refuse a stage-1 image the flash could not read"

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

Verdict: Follow-up review. This head is a pure re-stack onto #115's rebased tip — both of
this PR's own commits are byte-identical by git patch-id and there is no new work. Consequently
neither open finding has moved, and I re-measured both at this head rather than carrying them
forward on assertion: the one-line-if bypass in the guard still passes, and the stage-1 image is
still 0 bytes. Nothing new to report against the diff. What this PR needs is #115 to land and #138
to close the Critical.

Status of the previous findings

#	Prev. severity	Status	Evidence at this head
Critical (P0), from eBoot-129-4b42626f.md	Critical (P0) — stage-0 reports IMAGE_VALID and jumps having hashed zero bytes; eboot_firmware.bin links to 0 bytes	Open. Untouched by this PR, correctly. Re-measured, not recalled.	Cross-built at d68d5d1f with -DCMAKE_TOOLCHAIN_FILE=toolchains/arm-none-eabi.cmake -DEBLDR_BOARD=stm32f4 -DCMAKE_BUILD_TYPE=Release: configure rc 0, build rc 0, build-arm/ebldr_stage0.bin 12904 bytes, build-arm/eboot_firmware.bin 0 bytes. build-arm/stage1_hash.c:18 const uint32_t stage1_expected_size = 0u;, :6 SHA-256: e3b0c442…b855 — the digest of the empty string. grep -l ENTRY boards/*/*_stage1.ld → no matches. Identical to the measurement at 949775b3. #138 is the PR that addresses it; I checked its state directly — OPEN, MERGEABLE, updatedAt 2026-09-15T06:30:21Z — and issue #136 is OPEN. It is reviewed separately and blocking this PR on it would be wrong.
1, from eBoot-129-949775b3.md	Medium (P2) — a HAL result discarded inside a one-line if/while body still passes the guard	Open. Untouched.	Bypass reproduced against the module's own helpers at this head, driving _strip_comments → _statements_calling → _rest_of_enclosing_block → _result_is_examined exactly as test_every_hal_read_and_write_in_stage0_examines_its_result does: if (need) eos_hal_flash_read(0,0,0); → examined=True; while (n--) eos_hal_flash_read(0,0,0); → examined=True. Both discard the return value. The four controls still behave: inline condition → True, return read(…) → True, bare call → False, int rd = …; (void)rd; → False, int rc = …; if (rc != EOS_OK) → True. _result_is_examined at tests/unit/test_stage0_hal_results.py:74 is unchanged: re.match(r"(if|while)\s*\(", head) asks only whether the statement starts with if.

Findings 2, 3 and 4 from the first review were resolved at 949775b3 and re-verified there; they
are not restated. The build_sim/ observation is unchanged and still not charged to this PR.

Findings

#	Severity	File:line	Finding	Recommended fix
1	Medium (P2)	tests/unit/test_stage0_hal_results.py:74	Carried forward verbatim, re-measured above. Not reachable in stage0/ today — I re-enumerated the call sites and none is written as a one-line if/while body — so the guard is still correct about the current tree and this remains future-facing, exactly as the two bypasses closed in the previous round were.	Unchanged and still one line: re.match(r"(if|while)\s*\(", head) and head.count("(") > head.count(")"). Re-verified that it discriminates at this head: "if (" → True, "while (" → True, "if (eos_ok && " → True, "if (need) " → False, "while (n--) " → False, "return " → False (still caught by the return branch above it). Add the two negative cases beside the two already in test_the_guard_rejects_an_assigned_but_untested_result.

No new findings. There is no new source on this branch since the last review, and I did not go
looking for a second opinion to fill the table.

Verification performed for this review

Detached worktree at .ai/autoreview/state/scratch/eBoot-129 off refs/pull/129/head. The user's
eBoot checkout was not touched; nothing was committed or pushed. No probe was written to a tracked
file — the guard probes were driven through the module's helpers in a scratch interpreter, so the
worktree stayed clean throughout.

Check	Result
Is this head new work or a re-stack?	RE-STACK, confirmed. Two own commits at each head; the git patch-id --stable sets for 4b42626/949775b and 1de05d6/d68d5d1 are identical — the diff of the two sets is empty. The parent moved a276016 → e152d8e, which is #115's own rebase.
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.32s
EOS_REQUIRE_SIGNING_TESTS=1 pytest tests/ -q	PASS — 87 passed, 2.35s. The author's comment says 85; the two extra are tests/unit/test_sign_image.py cases that arrived on the new base from #117, not from this branch.
pytest tests/unit/test_stage0_hal_results.py -q	PASS — 5 passed
Cortex-M4 cross-compile via toolchains/arm-none-eabi.cmake, EBLDR_BOARD=stm32f4, Release	PASS — configure rc 0, build rc 0, ebldr_stage0.bin 12904 bytes. The changed stage0/jump_stage1.c compiles on the target toolchain, not just the host.
Critical re-measured	eboot_firmware.bin 0 bytes; stage1_hash.c:18 stage1_expected_size = 0u; :6 hash e3b0c442…b855; no ENTRY() in either *_stage1.ld. Unchanged from 949775b3.
Finding 1 bypass re-probed	if (need) read(…) → True ❌ · while (n--) read(…) → True ❌ · inline condition → True ✅ · bare → False ✅ · return read(…) → True ✅ · (void)rd → False ✅ · assigned-then-tested → True ✅
Proposed fix re-verified	Discriminates on all six head shapes as listed in the finding.
#138 / #136 state	#138 OPEN, MERGEABLE, updatedAt 2026-09-15T06:30:21Z; issue #136 OPEN.
Was anything weakened?	No. No new source since the last review; ctest 31/31 and the guard module 5/5 both unchanged.

Architecture conformance

The PR conforms; the surrounding code still does not, for the reason recorded in both previous
reviews. §21: eBoot is Tier 1 Foundation, every touched file is in the owning repo, §21.1 not
engaged. §5.1 dependency direction holds — stage0/ calls down into hal/, core/ and include/,
and EBLDR_FAIL_STAGE1_HASH/EBLDR_FAIL_STAGE1_READ living in include/eos_types.h moves private
literals into the contract header, which is the correct direction. §5.1's "eBoot keeps the trusted
computing base minimal and auditable" is what the commits serve: a verification failure that logged
a negative record, then wrote a positive one and jumped anyway, was not auditable. §8's boot flow —
Verify Manifest → Verify Image → … → Load EoS → Transfer Control — is honoured on this path;
control no longer reaches Transfer Control from a failed verify. §8.1's silence on the stage-0 →
stage-1 measurement is still the design gap behind the Critical, and the proposal appended during an
earlier review of this PR — "§8.1 is silent on the stage-0 → stage-1 measurement, so a build can
satisfy the whole secure-boot chain while measuring nothing" (2026-09-14) in
.ai/autoreview/proposals/2026-09.md — stands. No new proposal is appended; nothing about the
design gap has changed at this head.

Blocked / stale status

Stated plainly because the brief asks for it: this PR is green and correct, and waiting. No new
commits since the last review while one Medium finding remains open, so by the brief's definition it
is stale on that finding. It is blocked behind #115 (24 of the 27 files in the bundle diff
belong to that PR), and the Critical it sits next to is blocked behind #138. What would unblock
it: land #115, then either push the one-line guard fix here or record a decision not to.

Proposed changes

In this PR (one line and two assertions, unchanged from the last review):
  test_stage0_hal_results.py:74   and head.count("(") > head.count(")")
  test_stage0_hal_results.py      `if (need) read(...)` and `while (n--) read(...)`
                                  as negative cases beside the two already there

Not this PR, unchanged in priority:
  P0  stage-1 links to 0 bytes and its measurement verifies SHA-256("")  -> #138 / #136
  P3  cortex_r5 stage-0 is 0 bytes for the same class of reason          -> #137

No fix PR opened. tests/unit/test_stage0_hal_results.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. The finding is also Medium, below the brief's High bar for an
unattended fix PR.

Not checked

• No hardware, and no emulation of the boot path. Stage-0 was NOT run on a target or in
  QEMU, so the Critical's consequence remains Observed in the code and in the generated
  stage1_hash.c, not demonstrated on a device.
• cortex_r5 was not rebuilt this round. The stm32f4 measurement is Verified; that cortex_r5
  is in the same state is Inferred from its identically shaped linker script.
• The other board directories were not built. Only two have a *_stage1.ld, so I believe the
  blast radius is those two, but I have not proved no third path exists.
• release.yml's board builds were NOT run — they need a tag. That they set no toolchain file
  and therefore skip this block entirely is unchanged and still Inferred.
• The #115 half of the 27-file bundle diff was not reviewed here. See
  reports/eBoot-115-e152d8ed.md.
• The two own commits were not re-read line by line. They are patch-identical to the heads
  reviewed in eBoot-129-4b42626f.md and eBoot-129-949775b3.md; that is Inferred from patch-id,
  not re-observed.
• ASan/UBSan, Valgrind, fuzz, CodeQL, cppcheck/clang-tidy — NOT RUN locally. CI green on this
  head; job logs not read.
• #138's contents were not reviewed here — only its state and mergeability. It is reviewed
  separately.
• Mergeability at this head — NOT re-checked. It was BLOCKED at the first review, behind #115.

---

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

Thanks for re-measuring rather than carrying forward — and for catching that I had missed the one-line-body finding from the 949775b review entirely. e018474 (range now e152d8e..e018474): the head counts as inside the condition only when it has an unclosed parenthesis — re.match(r"(if|while)\s*\(", head) and head.count("(") > head.count(")"), your line exactly. if (need) eos_hal_flash_read(...) and while (n--) eos_hal_flash_read(...) are both refused now and sit in test_the_guard_rejects_an_assigned_but_untested_result beside the two from the previous round, with if (ok && eos_hal_flash_read(...) != EOS_OK) as the control that a call deeper in a condition still counts. pytest tests/ 87 passed. #138 is restacked onto this head (2482df0).