fix(recovery): refuse an unprovisioned secret, and a write into an unmapped slot

[Kartikey1306]

Stacked on #115 (master does not build without it). Review e152d8e..a9c6a2e for this change alone: three commits (restacked once, onto #115's e152d8e; a276016..184f431 was the same content on the previous base). 42abb8d (was 26ac2d4) is the fix -- core/recovery.c (+22/−2), tests/unit/test_recovery.c (+3 tests), CHANGELOG.md. a9c6a2e (was 184f431) fixes the guard's header parser (a define with a trailing comment was dropped silently; the match now stops at the value, with two tests). 72a4b1f (was 89e2242) answers the review's findings 1 and 3: include/eos_types.h (EOS_LOG_AUTH_SUCCESS/FAIL/UNPROVISIONED), core/recovery.c (uses them by name), tools/uart_recovery.py (0x22: "AUTH_UNPROVISIONED"), tests/unit/test_boot_log_event_names.py (new guard), CHANGELOG.md. #131 is stacked on this PR and closes finding 2.

Two checks in core/recovery.c that were weaker than the code around them says

1. An unprovisioned recovery secret authenticated anyone

recovery_handle_auth() read the shared secret from OTP offset 0x180 and compared the client's response against SHA-256(challenge || secret) with no look at the secret itself. Unprogrammed fuses read back as all zeros or all ones, and both are public: a device whose recovery secret was never provisioned authenticated any client that sent SHA-256(challenge || 00…00), and every destructive command (ERASE, WRITE, BOOT, FACTORY) was then open to it. core/keystore.c already refuses an all-zero key for exactly this reason.

Fix: both patterns are refused before the comparison — branch-free, so the check does not leak which value the fuses hold — counted as an authentication failure and logged as event 0x22.

Blast radius, stated exactly: no shipped board port under boards/ provides otp_read, so on every current board eos_hal_otp_read() returns EOS_ERR_NOT_SUPPORTED and authentication always failed. The defect was latent; the first board port with OTP would have shipped it.

2. A write into an unmapped slot landed at a flash address

recovery_handle_write() had stopped calling eos_recovery_write_in_range() — the rule tests/unit/test_recovery.c exercises, which refuses a slot whose base is 0 — and checked offset + len against the slot size inline. #55's replay did that while repairing a duplicate declaration. For a slot the board leaves unmapped, "write at offset 0x800 of slot B" became "write at flash address 0x800" — in the test layout, between the boot-control block and its backup.

Fix: the handler uses the shared rule again, so the wire has one definition and the tests test the code the wire hits.

Tests (Verified, each run against the unfixed handler first)

Test	Unfixed handler	Fixed
test_auth_refuses_an_unprovisioned_secret — all-zero and all-ones secret, correct response for that secret, then a WRITE	auth ACKed (out_buf[33] == RCVR_ACK)	auth NACKed, the write NACKed, flash untouched
test_auth_accepts_a_provisioned_secret — the control: same exchange, provisioned secret	passes	passes (so the refusal is about the value, not the exchange)
test_write_refuses_a_slot_the_board_leaves_unmapped — slot B with base 0, WRITE at offset 0x800	write ACKed, sim_flash[0x800] overwritten	NACKed, sim_flash[0x800] still 0xFF

Full suite on this head: ctest 31/31 (test_recovery 6/6), pytest 86 passed (80 + the 6 in the new guard).

Review follow-up (89e2242)

Finding	What changed
1 — 0x22 had no name and the client printed UNKNOWN(0x22)	include/eos_types.h defines EOS_LOG_AUTH_SUCCESS 0x20, EOS_LOG_AUTH_FAIL 0x21, EOS_LOG_AUTH_UNPROVISIONED 0x22 beside the other EOS_LOG_* codes; core/recovery.c logs by name at all four sites; tools/uart_recovery.py decodes 0x22. tests/unit/test_boot_log_event_names.py holds the three lists together: every eos_boot_log_append() in core/, stage0/, stage1/ passes an EOS_LOG_* name (no literal), every code the header defines is in BOOT_LOG_EVENT_NAMES under the same name, and the client names nothing the header does not define. Negative control against 26ac2d4: 3 of 4 fail -- the four bare literals at core/recovery.c:170/183/200/205, and 0x20/0x21 named by the client with no definition.
2 — tick-seeded LCG when rng_get is absent (pre-existing)	Not in this PR, as the review asks. Issue #130, fixed in #131, stacked on this head: the AUTH is refused (EOS_LOG_AUTH_NO_ENTROPY 0x23), with its own test and negative control. Added to Not changed below.
3 — a zero-length WRITE now NACKs	Stated in the CHANGELOG entry and below.

Not changed

RESET and INFO still need no authentication, and SHA-256(challenge || secret) is still the response construction (the file's own comment calls it a simplification of HMAC); neither is a bypass and both are outside this fix.

Two more, recorded rather than implied safe:

• The RNG fallback is a bypass and is not fixed here. When eos_hal_rng_get() fails, recovery_handle_auth() synthesises the challenge from an LCG seeded with the millisecond tick, and no board port provides rng_get (0 of 83), so that is the challenge on every board today. It is latent for the same reason as the unprovisioned-secret defect (no otp_read either), and it is closed in fix(recovery): issue no challenge on a board without an entropy source #131 on top of this PR.
• Wire change: routing WRITE through eos_recovery_write_in_range() also refuses len == 0, where the inline check had ACKed and written nothing. A client that sent empty writes sees a NACK now.

Closing issue

Fixes #126

[codecov-commenter]

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

Codecov Report

❌ Patch coverage is 96.00000% with 4 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
tests/unit/test_boot_log_event_names.py	97.14%	1 Missing and 1 partial ⚠️
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#127 "fix(recovery): refuse an unprovisioned secret, and a write into an unmapped slot"

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

Verdict: Both fixes are correct, minimal, and land in the right place — and unusually for a
security fix, the tests provably catch the defects: I reverted each change independently and the
matching test failed each time, then passed again when restored. Reviewed a276016..26ac2d4
(one commit) as the body asks; the rest of the 27-file diff belongs to #115. Three findings, none
of them in the fix itself: a new boot-log event code that the shipped recovery client cannot
decode, a fail-open RNG path eight lines above the fix that the body's "Not changed" list omits,
and an unannounced wire behaviour change for a zero-length WRITE.

Findings

#	Severity	File:line	Finding	Recommended fix
1	Medium (P2)	core/recovery.c:202; tools/uart_recovery.py:62-77	Event 0x22 is emitted but nothing can decode it — including this repo's own recovery client. The boot log is a serialized record read off the device (§8.1: crash/health information available to update logic), and tools/uart_recovery.py maps codes to names at :86, printing the result in the log command at :268. Its table ends at 0x21: "AUTH_FAIL". I ran the lookup: 0x20 → AUTH_SUCCESS, 0x21 → AUTH_FAIL, 0x22 → UNKNOWN(0x22). So the one signal that distinguishes "someone is guessing the secret" from "this device was never provisioned" — the exact condition this PR exists to detect — is the one a field engineer reading the boot log cannot name. It is also the only event code with no symbolic definition anywhere: include/eos_types.h:134-145 defines EOS_LOG_BOOT_START … EOS_LOG_BOOT_FAIL, and core/recovery.c bypasses that namespace for 0x20/0x21/0x22 with bare hex and a trailing comment. The 0x20/0x21 precedent is pre-existing; this PR adds the third instance, and tests/unit/test_uart_recovery.py:76 pins 0x21 without pinning the table against core/, so the drift is silent in both directions.	Two lines and a test: 0x22: "AUTH_UNPROVISIONED" in BOOT_LOG_EVENT_NAMES, and #define EOS_LOG_AUTH_UNPROVISIONED 0x22 (with 0x20/0x21) beside the existing EOS_LOG_* codes in include/eos_types.h, used by name at core/recovery.c:202. Then a test that asserts every eos_boot_log_append( literal in core/ appears in BOOT_LOG_EVENT_NAMES — the same shape as the suffix-coverage test in #122, and the thing that stops the next code from drifting too.
2	High (P1) — pre-existing, not introduced here; do not block this merge on it	core/recovery.c:141-149	When the RNG is unavailable the challenge is fabricated from a tick-seeded LCG, and that is the only path any current board takes. eos_hal_rng_get() failing falls through to seed = eos_hal_get_tick_ms(); seed = seed*1103515245 + 12345; challenge[i] = seed >> 16. hal/hal_core.c:172 returns EOS_ERR_NOT_SUPPORTED when the board provides no rng_get, and git grep rng_get boards/ returns nothing — no shipped board port provides one. A challenge exists for freshness, not secrecy; a predictable one makes a captured (challenge, response) pair replayable by an attacker who can reset the device and re-enter recovery, which is the recovery threat model exactly. This is the same failure shape as the bug the PR fixes, and .ai/security.md names it first: "A verification step that cannot run must fail, not pass. A HAL returning EOS_ERR_NOT_SUPPORTED is not success." I am recording it at its severity rather than rounding down, but flagging scope honestly: it is untouched by this commit, it is latent for the same reason the author documents for OTP (no board provides otp_read either, so authentication fails outright today), and closing it belongs in its own change. The reason it is here at all is that the body's "Not changed" section enumerates the RESET/INFO exemptions and the SHA-256(challenge ‖ secret) construction and correctly argues neither is a bypass — and omits this one, which is.	Refuse: if (rc != EOS_OK) { auth_fail_count++; eos_boot_log_append(<a new code>, EOS_SLOT_NONE, auth_fail_count); return recovery_send_nack(); }. A board with no entropy source has no authenticated recovery, and should say so rather than simulate one. Separate issue and separate PR; add it to the body's "Not changed" list in the meantime so it is recorded rather than implied safe.
3	Low (P3)	core/recovery.c:310	A zero-length WRITE now NACKs where it previously ACKed, and the body does not say so. The replaced inline test was slot_size == 0 || (uint64_t)offset + len > (uint64_t)slot_size, which for len == 0 is false — the handler ACKed, called eos_hal_uart_recv(buf, 0, …) and eos_hal_flash_write(base, buf, 0). eos_recovery_write_in_range() refuses len == 0 at core/recovery.c:281. The direction is safe and the new behaviour is the better one, so this is not a defect — but it is a change in what a recovery client sees on the wire, and brief item 8 is that silence about a behaviour change is itself the finding. Observed by reading both expressions, not executed.	One sentence in the PR body and the CHANGELOG.md entry: a WRITE with len == 0 is now refused.

Not a finding, recorded because it reads like one: the new early return at core/recovery.c:196-203
leaves shared_secret on the stack without running the volatile zeroing loop at :211-212. On
that path the buffer holds all zeros or all 0xFF by construction — there is nothing to disclose —
so the omission is harmless. The pre-existing OTP-read-failure return at :187-192 has the same
shape and additionally leaves the buffer uninitialised; also harmless, also untouched here.

What this PR gets right

The unprovisioned check is placed correctly: after the OTP read and before the secret reaches
eos_sha256_update(), so an unprovisioned device never computes a response at all. The accumulator
loop is genuinely branch-free over the secret's bytes and only the aggregate is branched on, which
is the right granularity — the two outcomes are distinguishable anyway. test_auth_accepts_a_provisioned_secret
is a real control rather than decoration: without it, test_auth_refuses_an_unprovisioned_secret
could pass because the scripted exchange was broken. And routing the write bounds back through
eos_recovery_write_in_range() means the wire, the unit tests (tests/unit/test_recovery.c:371)
and the fuzz harness (tests/fuzz/fuzz_recovery_protocol.c:41) now all exercise one definition —
the drift between them is what let #55's replay reintroduce this.

Verification performed for this review

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

Check	Result
cmake -B build/host -DEBLDR_BUILD_TESTS=ON -DCMAKE_BUILD_TYPE=Debug → cmake --build --parallel 4	PASS, no new warnings from this diff
ctest --test-dir build/host --output-on-failure --no-tests=error	PASS — 31/31, 10.97s — matches the body's claim
eboot_test_recovery	PASS — 6/6 (3 before this commit)
Negative control 1 — if (all_zero == 0 || all_ones == 0xFF) forced to if (0), rebuilt	test_auth_refuses_an_unprovisioned_secret FAILS at test_recovery.c:299 (out_buf[33] == RCVR_NACK). The test catches the defect it claims to.
Negative control 2 — the old inline slot_size == 0 || offset + len > slot_size restored, rebuilt	test_write_refuses_a_slot_the_board_leaves_unmapped FAILS at test_recovery.c:364 (out_buf[34] == RCVR_NACK); the unprovisioned tests still pass, so the two fixes are independently pinned.
Both reverts restored, rebuilt	6/6 pass; git status clean
Boot-log decode of the new event, via tools/uart_recovery.py	0x20 → AUTH_SUCCESS, 0x21 → AUTH_FAIL, 0x22 → UNKNOWN(0x22) — finding 1
git grep rng_get boards/, git grep otp_read boards/	both empty — confirms the author's latency argument for OTP, and establishes finding 2
eos_recovery_write_in_range() read against the removed inline check	Adds base == 0 (the fix) and len == 0 (finding 3); wrap and underflow handling unchanged
Placement of the new check relative to eos_sha256_update(&ctx, shared_secret, …)	Correct — the secret is refused before it is consumed

Architecture conformance

Conforms. §21: eBoot is Tier 1 Foundation and every file touched is inside the owning repo; no new
repository is implied, so §21.1 is not engaged. §5.1 dependency direction is untouched —
core/recovery.c calls downward into hal/ and include/, and nothing points up a tier. §5.1's
"eBoot keeps the trusted computing base minimal and auditable" is served in both directions here:
the write path now has one definition of its bounds rule instead of two that had already drifted
apart once, which is auditability, and deleting an inline duplicate of a security check is the same
move #122 makes for the artifact scan. §8.1 ("factory/recovery image strategy"; "crash/health
information available to update logic") is what finding 1 is measured against — a health record
whose codes are defined in no header and decoded by no tool is not available to update logic in any
useful sense. §14.1's hardware-root-of-trust clause is what finding 2 is measured against: a target
without hardware support is required to degrade to a documented, weaker posture, not to silently
synthesise the strong one from a tick counter.

Proposed changes

In this PR (small, and the PR is the natural place):
  tools/uart_recovery.py   0x22: "AUTH_UNPROVISIONED" in BOOT_LOG_EVENT_NAMES
  include/eos_types.h      EOS_LOG_AUTH_{SUCCESS,FAIL,UNPROVISIONED} beside the
                           existing EOS_LOG_* codes; use them in core/recovery.c
  CHANGELOG.md / body      say that a zero-length WRITE is now refused     (finding 3)

Separate issue, not this PR:
  core/recovery.c:141-149  refuse when eos_hal_rng_get() fails, instead of
                           falling back to a tick-seeded LCG               (finding 2)
  tests/                   a test asserting every eos_boot_log_append() literal
                           in core/ is present in BOOT_LOG_EVENT_NAMES

No fix PR opened. Finding 1's fix has to land alongside the 0x22 emission, which exists only
on this branch, so a branch cut from origin/master — what fix-start.sh produces — would add a
name for an event nothing emits. Finding 2 is on origin/master and would qualify, but it is not
the "small and provable" kind: refusing where the code currently falls back changes recovery
behaviour on every board, and I cannot verify the consequences without hardware. It belongs to a
human.

Not checked

• No hardware. Every statement about recovery is from the host simulation in
  tests/unit/test_recovery.c (sim_ops, sim_flash). The UART framing, timing, and the real
  eos_hal_otp_read() on a board with fuses were NOT exercised — no board port provides
  otp_read at all, so no such path exists to exercise yet.
• Finding 2's replay is reasoned, not demonstrated. I did not build a replay against a device
  or measure how many distinct challenges the LCG produces across realistic tick_ms values at
  reset. The claim I am making is the narrow one: the fallback is deterministic given the tick, and
  it is the only path a current board takes. The step from there to a practical replay is
  Inferred.
• tests/fuzz/fuzz_recovery_protocol.c was NOT run. I read it to confirm it drives the same
  helper; I did not build the fuzz harness or run a campaign.
• The #115 half of the diff was not reviewed here — core/sha512.c, core/ed25519_verify.c,
  tools/gen_fw_update_test_sigs.py and the workflow changes are 24 of the 27 files and belong to
  that PR. This review covers a276016..26ac2d4 only. If #115 changes, this PR's ctest result
  above no longer describes what will merge.
• ASan/UBSan, Valgrind, cross-compile, CodeQL, cppcheck/clang-tidy — NOT RUN locally. CI green
  on this head; logs not read.
• mergeStateStatus is BLOCKED — reviewDecision: REVIEW_REQUIRED plus the #115 dependency.
  I did not verify #115 is mergeable.

---

Automated architecture review of 26ac2d445224 — 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 — all three taken, in 89e2242 on this branch (range is now a276016..89e2242; the body's top line and a "Review follow-up" table say what each commit is).

Finding 1 — 0x22 had no name and the client printed UNKNOWN(0x22). Done the way you laid it out. include/eos_types.h now defines EOS_LOG_AUTH_SUCCESS 0x20, EOS_LOG_AUTH_FAIL 0x21 and EOS_LOG_AUTH_UNPROVISIONED 0x22 beside the existing EOS_LOG_* codes; core/recovery.c logs by name at all four sites (the two pre-existing bare 0x20/0x21 included); tools/uart_recovery.py gains 0x22: "AUTH_UNPROVISIONED". The guard is tests/unit/test_boot_log_event_names.py, and it pins a little more than the one direction you asked for: every eos_boot_log_append() in core/, stage0/ and stage1/ must pass an EOS_LOG_* name rather than a literal, every code the header defines must appear in BOOT_LOG_EVENT_NAMES under the same name, and the client may not name a code the header does not define. Negative control against 26ac2d4 (header, recovery.c and the client restored): 3 of 4 fail — the four bare literals at core/recovery.c:170/183/200/205, and 0x20/0x21 named by the client with no definition behind them. pytest tests is 84 (80 + 4).

Finding 2 — the tick-seeded LCG. Agreed on every point, including the scope: it is not in this PR. It is issue #130 and PR #131, stacked on this head, which refuses the AUTH outright when eos_hal_rng_get() fails (NACK, EOS_LOG_AUTH_NO_ENTROPY 0x23 — named, decoded, and covered by the same guard — counted as a failure so the backoff and the RCVR_MAX_AUTH_FAILS cap apply). Its test copies sim_ops with rng_get = NULL, the shape of every real board, and fails against the old code at the first byte of the reply. I had measured the same thing you did (git grep rng_get boards/ empty; 0 of the 83 ops tables) and opened it twelve minutes after your review landed. The body's Not changed list now names this bypass explicitly and points at #131 instead of implying it safe.

Finding 3 — a zero-length WRITE now NACKs. Stated in the CHANGELOG entry and in the body's Not changed section as a wire change: eos_recovery_write_in_range() refuses len == 0 where the inline check had ACKed and written nothing.

On the not a finding: agreed the early return leaves an all-zero/all-FF buffer, which is nothing to disclose; left as is.

Full suite on 89e2242: ctest 31/31, test_recovery 6/6, pytest 84. Line endings held (core/recovery.c 545 CR bytes before and after; include/eos_types.h 169→175, one per added line).

[srpatcha]

Review — eBoot#127 "fix(recovery): refuse an unprovisioned secret, and a write into an unmapped slot"

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

Verdict: Follow-up review of 26ac2d44..89e2242c (one commit). All three findings from the
previous review are resolved, and the guard the author wrote for finding 1 is stricter than the
one I asked for — it pins three directions instead of one. The author's negative control reproduced
exactly. One new finding against that guard, Medium: its header parser anchors on end-of-line, so an
EOS_LOG_* define written in this header's own existing style — with a trailing comment — drops out
of the client cross-check silently. I demonstrated it with EOS_LOG_AUTH_NO_ENTROPY 0x23, which is
the very next code #131 adds on top of this head.

Previous findings — status

#	Previous finding	Status	Evidence
1	Medium (P2) — event 0x22 emitted as a bare literal, decoded by nothing, including this repo's own recovery client	Resolved in 89e2242	include/eos_types.h:146-151 defines EOS_LOG_AUTH_SUCCESS/FAIL/UNPROVISIONED beside the other EOS_LOG_* codes; core/recovery.c:185,202,219,224 log by name at all four sites, including the two pre-existing bare 0x20/0x21; tools/uart_recovery.py:77 gains 0x22: "AUTH_UNPROVISIONED". The guard tests/unit/test_boot_log_event_names.py pins more than I asked: no bare literal at any eos_boot_log_append() in core//stage0//stage1/, every header code named by the client under the same name, and no client entry without a define behind it. Negative control reproduced: restoring the three files to 26ac2d4 gives 3 failed, 1 passed, exactly the count the author reported.
2	High (P1), pre-existing — eos_hal_rng_get() failure falls back to a tick-seeded LCG, the only path any current board takes	Correctly scoped out, and now recorded rather than implied safe	The body's Not changed list names the bypass and points at issue #130 / PR #131 instead of omitting it, which is what the finding asked for. The defect itself is untouched here and remains open on origin/master; core/recovery.c:141-149 at this head is byte-identical to 26ac2d4. #131 is in this same review batch and I review it separately — this PR is not the place to close it, and blocking on it would be wrong.
3	Low (P3) — a zero-length WRITE now NACKs where it previously ACKed, unannounced	Resolved in 89e2242	CHANGELOG.md:6 now states it in the entry itself: "which also refuses a WRITE with len == 0, where the inline check had ACKed it and written nothing; a recovery client sees a NACK there now". Also in the body's Not changed section.

The not a finding from last time — the early return at core/recovery.c:199-204 skipping the
volatile zeroing loop — is unchanged, still harmless for the same reason, and the author agreed on
the thread. Not restated.

Findings

#	Severity	File:line	Finding	Recommended fix
1	Medium (P2)	tests/unit/test_boot_log_event_names.py:40	The guard's header parser requires the #define to end at the hex value, so any event code written with a trailing comment is invisible to it — and this header's existing style puts comments there. header_events() matches r"^\s*#define\s+EOS_LOG_([A-Z0-9_]+)\s+(0x[0-9A-Fa-f]+)\s*$". EOS_LOG_MAGIC at include/eos_types.h:50 is already written 0x454C4F47 /* "ELOG" */, so the style is in the file the test reads. Demonstrated, not argued: I added #define EOS_LOG_AUTH_NO_ENTROPY 0x23 /* board provides no rng_get */ to eos_types.h, deliberately left tools/uart_recovery.py alone, and ran the module — 4 passed. A new event that the field client decodes as UNKNOWN(0x23) sails through the guard whose entire purpose is to stop that. This is not hypothetical: the author states on the thread that #131 adds exactly EOS_LOG_AUTH_NO_ENTROPY 0x23, and it is in this same batch. The guard catches a commented define today only by accident — core/recovery.c emits EOS_LOG_AUTH_UNPROVISIONED by name, so rule 1 trips on the missing define — and that incidental cover disappears for any code defined but not yet emitted from those three directories. I confirmed the narrower half separately: comment out AUTH_UNPROVISIONED and delete 0x22 from the client table and test_client_names_every_event_the_header_defines still passes; only the two rules that mention the name explicitly fail.	Drop the end anchor and stop at the value: r"^\s*#define\s+EOS_LOG_([A-Z0-9_]+)\s+(0x[0-9A-Fa-f]+)\b". Then add the case as a test — write a temp header with a commented define and assert header_events() still finds it — because the failure mode here is under-collection, and a parser that silently returns fewer rows than the file contains cannot fail loudly on its own. The existing assert events at :42 only catches finding nothing. Worth doing in this PR: it is one character of regex plus a test, and #131 is the first thing that will hit it.

Verification performed for this review

Detached worktree at .ai/autoreview/state/scratch/eBoot-127 on 89e2242c. 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. 26ac2d44..89e2242c was not present locally, so the PR
head was fetched into refs/autoreview/scratch127 read-only. Every probe below was reverted and the
worktree confirmed clean (git status --short empty) before the next one.

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 — 31/31, 11.07s — matches the body's claim
eboot_test_recovery	PASS — 6/6, including both fixes' tests
pytest tests/ -q	PASS — 84 passed, 1.84s — matches the body's claim of 84 (80 + 4)
Negative control, author's claim re-run — include/eos_types.h, core/recovery.c, tools/uart_recovery.py restored to 26ac2d4	3 failed, 1 passed — the exact count reported. The four bare literals and the two client-named codes with no define are both caught.
Probe — commented define, client not updated (finding 1)	EOS_LOG_AUTH_NO_ENTROPY 0x23 /* … */ added, client table untouched → 4 passed. The guard does not see it.
Probe — commented define + 0x22 removed from the client table	test_client_names_every_event_the_header_defines passes; only the two rules naming AUTH_UNPROVISIONED literally fail. Confirms the client-table rule is what goes silent.
header_events() coverage at this head	15 of 15 EOS_LOG_* event codes collected; EOS_LOG_MAGIC correctly excluded by name. No event is currently missed.
Every eos_boot_log_append() call site vs FIRMWARE_DIRS	All 35 firmware call sites are in top-level core/, stage0/, stage1/ .c files; none in hal/, boards/ or a subdirectory. The glob("*.c") is sufficient for the tree as it stands.
core/recovery.c:141-149 (the finding-2 RNG fallback) vs 26ac2d4	Byte-identical. Untouched by this commit, as the body says.
New #define values vs existing codes	0x20-0x22, no collision with 0x01-0x0C or EOS_LOG_MAGIC

Architecture conformance

Conforms; re-checked against the new commit rather than carried over. §21: eBoot is Tier 1
Foundation, and include/, core/, tools/, tests/unit/ are all inside the owning repo — §21.1
not engaged. §5.1 dependency direction is untouched and, notably, improved in the right direction:
core/recovery.c now depends on include/eos_types.h for its event vocabulary instead of carrying
private literals, and include/ still depends on nothing. §8.1's "crash/health information
available to update logic" is the clause the whole commit serves — three codes that were legible
only to someone reading core/recovery.c with a hex editor beside them are now named in the
contract header and printed by the client. Finding 1 is measured against the same clause: a guard
that lets the next code through silently leaves §8.1 satisfied by luck. §28's evidence policy is
honoured — the author's negative-control count and both suite totals reproduced exactly.

Proposed changes

In this PR (one character and a test, and #131 is the first caller that needs it):
  test_boot_log_event_names.py:40   ...(0x[0-9A-Fa-f]+)\b   instead of  ...\s*$
  test_boot_log_event_names.py      a case asserting a commented #define is still collected

Already agreed, tracked, not this PR:
  core/recovery.c:141-149           refuse when eos_hal_rng_get() fails   -> #130 / #131

This PR remains blocked behind #115 (a276016 and below are that PR's commits, 24 of the 27
files in the bundle diff), and mergeStateStatus was BLOCKED at the previous review. I did not
re-check mergeability at this head.

No fix PR opened. tests/unit/test_boot_log_event_names.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.

Not checked

• No hardware. Everything about recovery comes from the host simulation in
  tests/unit/test_recovery.c (sim_ops, sim_flash). Real UART framing, timing, and a real
  eos_hal_otp_read() on a board with fuses were NOT exercised — no shipped board port provides
  otp_read, so no such path exists to exercise.
• tools/uart_recovery.py was not executed against a device, only parsed. The log command's
  rendering of AUTH_UNPROVISIONED is Inferred from the table, not observed on a wire.
• The #115 half of the diff was not reviewed here — core/sha512.c, core/ed25519_verify.c,
  include/eos_boot_log.h, tools/gen_fw_update_test_sigs.py and the workflow changes belong to
  b7d7cac..a276016. This review covers 26ac2d44..89e2242c only. If #115 changes, the ctest and
  pytest numbers above stop describing what will merge.
• tests/fuzz/fuzz_recovery_protocol.c — NOT built or run.
• ASan/UBSan, Valgrind, cross-compile, CodeQL, cppcheck/clang-tidy — NOT RUN locally. CI green
  on this head; logs not read.
• Unrelated pre-existing defect observed, not charged to this PR: tests/unit/test_boot_log.c:303
  is printf("\n%d/%d tests passed\n", tests_passed) — two conversions, one argument, so the second
  reads an indeterminate vararg. -Wformat= flags it on every host build. It is on origin/master
  and in a file this PR does not touch. Recorded here so it is not lost.

---

Automated architecture review of 89e2242cd3e6 — 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, in 184f431 (range now a276016..184f431). You are right that the parser's failure mode is under-collection, which nothing downstream can report, and that #131's EOS_LOG_AUTH_NO_ENTROPY is the first thing that would have hit it.

header_events() now stops at the value ((0x[0-9A-Fa-f]+)\b, no end anchor) and takes an optional text so it can be tested on a sample. Two tests: test_header_parser_sees_a_define_with_a_trailing_comment parses a four-line sample with a bare define, a /* */-commented one, a //-commented one and EOS_LOG_MAGIC, and asserts the exact map (MAGIC excluded); test_header_parser_collects_every_event_define_in_the_real_header cross-checks the strict pattern against a looser count of #define EOS_LOG_* lines in the real header, so any future style the strict pattern misses shows up as a mismatch rather than a silent drop. Your probe re-applied on this head — #define EOS_LOG_AUTH_NO_ENTROPY 0x23 /* board provides no rng_get */ with the client untouched — now fails the guard where it should: tools/uart_recovery.py BOOT_LOG_EVENT_NAMES lacks: ['EOS_LOG_AUTH_NO_ENTROPY = 0x23']. pytest tests/ 86 passed (84 + 2). #131 will be restacked onto this head.

[srpatcha]

Review — eBoot#127 "fix(recovery): refuse an unprovisioned secret, and a write into an unmapped slot"

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

Verdict: Follow-up review. The one finding from the review of 89e2242c is resolved, and I
closed it with the same probe that opened it: the commented EOS_LOG_AUTH_NO_ENTROPY 0x23 define
that used to sail through the guard now fails it by name. The earlier two commits are patch-identical
after the re-stack onto #115's rebased tip; a9c6a2e is the only new work, it is test-only, and it
is clean. No new findings.

Previous finding — status

#	Previous finding	Status	Evidence
1	Medium (P2) — header_events() anchored its pattern on end of line, so an EOS_LOG_* define written in this header's own style (trailing comment) dropped out of the client cross-check silently	Resolved in a9c6a2e	The pattern is now EVENT_DEFINE = re.compile(r"^\s*#define\s+EOS_LOG_([A-Z0-9_]+)\s+(0x[0-9A-Fa-f]+)\b", re.M) — the end anchor is gone, exactly as recommended — and header_events() takes an optional text so it is testable on a sample. Both halves of the recommended fix landed, and the second one (a test for the under-collection case) is done twice over: test_header_parser_sees_a_define_with_a_trailing_comment asserts the exact map for a four-line sample carrying a bare define, a /* */-commented one, a //-commented one and EOS_LOG_MAGIC; test_header_parser_collects_every_event_define_in_the_real_header cross-checks the strict pattern against a looser #define EOS_LOG_* count over the real header, so a style the strict pattern misses surfaces as a mismatch rather than a silent drop. The review's own probe re-applied at this head: added #define EOS_LOG_AUTH_NO_ENTROPY 0x23 /* board provides no rng_get */ to include/eos_types.h, left tools/uart_recovery.py untouched → 1 failed, 5 passed, failing at test_boot_log_event_names.py:101 with tools/uart_recovery.py BOOT_LOG_EVENT_NAMES lacks: ['EOS_LOG_AUTH_NO_ENTROPY = 0x23']. That is a 4 passed → 1 failed flip on the identical probe. Header restored; git diff --stat clean.

The two findings closed at 89e2242c (event names, zero-length WRITE in the CHANGELOG) and the
one correctly scoped out to #131 (the tick-seeded LCG on eos_hal_rng_get() failure) are unchanged
and are not restated. #131 is in this same batch and is reviewed separately.

Findings

#	Severity	File:line	Finding	Recommended fix
—	—	—	None.	—

a9c6a2e is +34/−4 in one test file. I went looking for a hole in the new guard — a decimal
value, a parenthesised value, a define the strict pattern would miss — and in each case the
loose-vs-strict cross-check turns it into a failure rather than a silent drop, which is the property
the finding asked for. Not manufacturing a second opinion to fill the table.

Verification performed for this review

Detached worktree at .ai/autoreview/state/scratch/eBoot-127 off refs/pull/127/head. The user's
eBoot checkout was not touched; nothing was committed or pushed. Every probe was reverted and the
worktree confirmed clean before the next step.

Check	Result
What is new since 89e2242c?	One commit. git patch-id --stable over this PR's own commits: 26ac2d4/89e2242 are identical to 42abb8d/72a4b1f; a9c6a2e is the only addition. 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.65s
eboot_test_recovery	PASS — 6/6
EOS_REQUIRE_SIGNING_TESTS=1 pytest tests/ -q	PASS — 88 passed, 2.56s. The comment says 86; the two extra are tests/unit/test_sign_image.py cases that arrived on the new base from #117, not from this branch.
Probe from the previous review, re-applied verbatim	1 failed, 5 passed — test_client_names_every_event_the_header_defines fails with the exact message the author quotes. At 89e2242c the same probe gave 4 passed. This is the evidence the finding is closed.
Does the new guard fail loud on styles the strict pattern misses?	Yes, by construction — test_header_parser_collects_every_event_define_in_the_real_header compares sorted(header_events()) against a looser #define EOS_LOG_* name scan, so a decimal, parenthesised or otherwise-unmatched value becomes a mismatch. Checked by reading the assertion, not by mutating the header for each style.
Was anything weakened?	No. a9c6a2e is additive: one regex narrowed in scope of anchoring only (it now matches strictly more input), one docstring, one optional parameter, two tests added. No test disabled, no assertion removed, no lint loosened, no permission widened. Suite totals moved up: pytest 84 → 86 on this branch's own count.

Carried observation, not a finding and not this PR's. The previous review recorded that
tests/unit/test_boot_log.c:303 is printf("\n%d/%d tests passed\n", tests_passed) — two
conversions, one argument. It is still on origin/master, unchanged, and this build warns on it
(test_boot_log.c:303:19: warning: format '%d' expects a matching 'int' argument [-Wformat=]). What
is new is that I ran the binary: it prints 11/762734247 tests passed, and the denominator is an
indeterminate vararg that differs between runs. It is cosmetic rather than dangerous — ASSERT()
calls exit(1), so the suite does fail closed and ctest cannot be fooled by it — which is why it
stays Low and why no fix PR was opened for it here. It belongs to master, not to this PR.

Architecture conformance

Conforms; re-checked against the new commit rather than carried over. §21: eBoot is Tier 1
Foundation and tests/unit/ is inside the owning repo; §21.1 is not engaged. §5.1 dependency
direction is untouched — the commit changes one Python test module and adds no #include, link
line, CMake entry or manifest dependency of any kind. §8.1's "crash/health information available to
update logic" is the clause the PR as a whole serves, and this commit protects it: the guard that
keeps every boot-log event code named in the contract header and decoded by the field client is now
able to see the codes it is supposed to police. §28's evidence policy is honoured — the author's
claimed probe result and both suite totals reproduced, with the pytest count differing only by the
two tests the new base contributed.

Proposed changes

None. Merge after #115.

Already agreed, tracked, not this PR:
  core/recovery.c:141-149   refuse when eos_hal_rng_get() fails   -> #130 / #131
                            (#131 is stacked on this head and reviewed separately)

Not checked

• No hardware. Everything about recovery comes from the host simulation in
  tests/unit/test_recovery.c (sim_ops, sim_flash). Real UART framing, timing and a real
  eos_hal_otp_read() on a board with fuses were NOT exercised — no shipped board port provides
  otp_read, so no such path exists to exercise.
• tools/uart_recovery.py was not executed against a device, only parsed. Its rendering of
  AUTH_UNPROVISIONED on a wire remains Inferred from the table.
• The #115 half of the bundle diff was not reviewed here; this review covers
  89e2242c..a9c6a2e3 plus the re-stack check. See reports/eBoot-115-e152d8ed.md.
• tests/fuzz/fuzz_recovery_protocol.c — NOT built or run.
• ASan/UBSan, Valgrind, cross-compile, CodeQL, cppcheck/clang-tidy — NOT RUN locally. CI green
  on this head; job logs not read.
• The earlier two commits were not re-read line by line. They are patch-identical to the heads
  reviewed in eBoot-127-26ac2d44.md and eBoot-127-89e2242c.md; that is Inferred from patch-id,
  not re-observed.
• Mergeability at this head — NOT re-checked. It was BLOCKED at the first review, behind #115.

---

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

Acknowledged — thanks for re-running the probe on a9c6a2e rather than taking the description's word for it. Nothing further on this PR: the range stays e152d8e..a9c6a2e (restacked once onto #115's e152d8e), #131 is stacked on this head, and the stack is ready to merge behind #115.