[Tanisha1723]

Summary

Fix eBoot firmware update handling so the authenticated TLV anti-rollback counter is correctly consumed and enforced, while ensuring transport framing cannot report incomplete transfers as successful or accept unbounded data after the update container is complete.

The update path now treats the authenticated TLV area as part of the complete firmware container and keeps transport-level framing separate from the container's actual byte requirements.

Changes

Anti-rollback

• Read the minimum security version from the authenticated EOS_TLV_MIN_SEC_VER TLV rather than using the image version field.
• Verify the authenticated TLV counter against the hardware rollback floor.
• Stage the authenticated counter before boot in the jump path.
• Reject images whose authenticated security counter is below the hardware floor.
• Remove the obsolete eos_image_check_rollback() path.
• Add regression coverage proving that an image version can no longer bypass the hardware anti-rollback floor when the authenticated TLV counter is too low.

Streaming TLV handling

• Stream the TLV area through the firmware update state machine instead of assuming it arrives together with the payload.
• Track TLV bytes written and the total TLV length.
• Transition to verification only after the complete TLV area has arrived.
• Reject incomplete TLV data during finalization.
• Validate header, payload, and TLV lengths with overflow-safe container-boundary checks.
• Reject writes that arrive after the update container has already completed.

Transport completeness — XMODEM, YMODEM and raw

• Expose the update container's remaining byte requirement through eos_fw_update_bytes_wanted().
• Keep XMODEM/YMODEM protocol block sizes independent from the actual container size.
• Continue allowing padding within the final protocol block when the container completes part-way through the block.
• Reject an entire new XMODEM block once the update container is already complete instead of ACKing and silently discarding it.
• Reject an entire new YMODEM data block once the update container is already complete.
• Send CAN and return EOS_ERR_INVALID when an extra post-container XMODEM/YMODEM block is received.
• Reject XMODEM and YMODEM EOT when the container is still incomplete.
• Keep YMODEM's declared file_size as the protocol/framing bound while the update container remains the authority for actual bytes written.
• Reject a raw transfer that ends before the complete update container is written, sending UART_RAW_NAK and returning EOS_ERR_INVALID.
• Prevent progress reporting from reaching 100% while TLV bytes are still outstanding by including the TLV area in the total container size.
• Define the transport receive contract so EOS_OK means the complete [header][payload][TLV area] container has been written.
• Remove the obsolete no-op YMODEM branch that could otherwise allow completed-container writes to proceed without rejection.

Compatibility

• XMODEM final blocks may still contain protocol padding; bytes beyond the container are discarded as before.
• A YMODEM declared file size larger than the update container remains supported; surplus bytes inside the final protocol block are ignored.
• A new complete XMODEM/YMODEM data block arriving after the container is complete is now rejected rather than ACKed.
• XMODEM/YMODEM transfers that terminate with EOT before the container is complete now return EOS_ERR_INVALID.
• A raw transfer whose declared length is shorter than the complete update container is now rejected with UART_RAW_NAK and EOS_ERR_INVALID instead of the previous UART_RAW_ACK and EOS_OK.
• YMODEM's protocol-level declared file size remains unchanged; the container length remains authoritative for actual firmware data.

Testing

Targeted host tests:

• test_fw_update — 6/6 passed
• test_tlv_auth — 9/9 passed
• test_jump_app — 2/2 passed
• test_fw_transport — 19/19 passed
• test_rollback — 14/14 passed

Additional regression coverage verifies:

• Authenticated TLV counter is used for anti-rollback enforcement.
• Streaming TLVs do not transition to verification prematurely.
• Progress remains below 100% while TLV data is outstanding.
• Complete TLV data reaches verification and 100% progress.
• XMODEM rejects an extra block after container completion.
• YMODEM rejects an extra block after container completion.
• XMODEM rejects EOT before the container is complete.
• YMODEM rejects EOT before the container is complete.
• Raw transport rejects a transfer shorter than the complete update container.
• Final-block protocol padding remains compatible.

git diff --check passes for the submitted changes.

Build / CI Status

The targeted host tests pass as listed above.

The repository-wide CMake build is currently blocked by pre-existing issues on the base branch, including:

• duplicate point_is_identity definition in core/ed25519_verify.c
• existing __builtin_add_overflow MSVC warning/error handling
• /std:clatest requirement in core/keystore.c
• stale reserved[] static assertions in include/eos_image.h

These issues are unrelated to the changes in this PR and are tracked separately.

The ARM cross-build has also been verified successfully during review.

Scope

Out of scope: TLV counter emission. This PR implements and validates the consumer and enforcement side of the authenticated TLV anti-rollback counter — parsing the TLV area, binding it to the signed header hash, enforcing the hardware floor at finalize and at jump, and streaming the TLV area through the update path. The repository's image-packaging and signing tools (tools/imgpack.py, tools/eos_sign.py) do not yet emit EOS_TLV_MIN_SEC_VER, so no in-tree tool currently produces an image carrying the counter. Adding TLV emission and round-trip packer coverage is separate follow-up work, and hardware validation of the end-to-end counter lifecycle is therefore outside this PR's scope.

The remaining rollback-code duplication and dedicated test-infrastructure cleanup are also separate follow-up work.

Follow-up

The following items are intentionally left for separate issues/PRs:

• Add TLV counter emission to the image packaging/signing tools and round-trip packer tests.
• Consolidate duplicated rollback read/verify/stage logic across boot/update paths.
• Address the repository-wide test/build infrastructure issues tracked separately.
• Add broader hardware/end-to-end validation of the authenticated counter lifecycle.
• Update the remaining stale documentation listings separately where they do not describe the transport completeness contract.

Transport contract documentation

The transport receive framing only bounds the transfer; the firmware update container remains authoritative for completeness.

Returning EOS_OK while eos_fw_update_bytes_wanted() is nonzero is a contract violation. EOS_OK means the complete [header][payload][TLV area] update container has been written.

[srpatcha]

Review — eBoot#103 "Fix anti-rollback to use authenticated TLV counter"

head: c9fe84c author: Tanisha1723 ci: none reported

Verdict: Correct and valuable fix to a real anti-rollback bypass on the boot path that
actually runs. The core change conforms to §8.1 and .ai/security.md. What is missing is
coverage of the two production paths it changes, and cleanup of the defective function it
leaves exported.

Findings

#	Severity	File:line	Finding	Recommended fix
1	Medium	core/image_verify.c:230	After this PR eos_image_check_rollback() has no production caller — git grep at this head finds only the new test. It remains exported, and it is exactly the defect being removed: it compares a firmware image_version against the OTP monotonic floor. Any future caller reintroduces the bypass, silently and with a plausible-looking name.	Delete it. If the regression test needs to demonstrate the old behaviour, move the comparison into the test file under a name that says it is wrong (legacy_broken_version_floor_check), so no production translation unit can reach it.
2	Medium	tests/unit/test_tlv_auth.c:304	The new regression test does not exercise either changed path. It calls eos_rollback_read_image_counter() and eos_rollback_verify() directly. Neither eboot_jump_to_app() (stage1/jump_app.c:60) nor eos_fw_update_finalize() (core/fw_update.c:191) is invoked. I confirmed this by reading the test: reverting either production hunk leaves the suite green. The bug being fixed lives in the wiring, and the wiring is untested.	Add one test that drives eos_fw_update_finalize() to completion with an image carrying image_version = 0x00010000 and MIN_SEC_VER = 3 against sim_counter = 9, asserting EOS_ERR_ANTI_ROLLBACK. test_fw_update.c already has the context construction needed.
3	Medium	stage1/jump_app.c:60, core/secure_boot.c:148	Two independent implementations of the same TCB sequence now exist: eboot_main() → eboot_jump_to_app() (stage1/main.c:75, the path stage 1 actually takes) and eos_secure_boot(), which nothing in stage1/ calls. The PR comment acknowledges this — "Same sequence as eos_secure_boot() step 5b" — and duplication is how the two paths diverged in the first place: one got the TLV counter, the other kept image_version.	Extract the read → verify → stage sequence into one function in core/rollback.c (e.g. eos_rollback_gate_image(addr, bool stage)) and have both call sites use it, so the next change cannot land in one path only. Out of scope for this PR; open a follow-up.
4	Low	tests/unit/test_tlv_auth.c:339	tests_run is still a hand-maintained literal. This PR corrects it to 8, but the next test added under-reports unless someone remembers, and tests_passed == tests_run then passes with a test that never ran.	Already addressed org-wide by open PR eBoot#95 ("derive the suite totals … instead of restating them"). No change needed here; rebase after #95 lands.
5	Low	PR body, "Testing"	The body reports the C suite as unrunnable due to "the current MSVC build". The blocking errors are not MSVC-specific and not a Windows artifact — they are two merge defects on master itself that break the gcc host build too (see Not checked). Worth correcting so the next reader does not go looking for a toolchain problem.	Restate as: blocked by broken master, tracked by eBoot#94 / #98.

Note on the "Testing" section: the body states plainly that the suite could not be run,
rather than asserting an unsupported PASS. That is the right call and is not a finding.

Architecture conformance

Conforms. Master design §8.1 requires "rollback protection where hardware/policy supports
it"; .ai/security.md ("Boot and update chain") requires "anti-rollback counters actually
checked". Before this PR the counter was not actually checked on the live boot path —
hdr.image_version (scale 0x00MMmmpp) was compared against the OTP monotonic floor
(small integers), so an old image with a high version number cleared a high floor. The
change substitutes the authenticated EOS_TLV_MIN_SEC_VER, which core/rollback.c binds
to the signed header via tlv_len/tlv_hash. That is the value §8.1 intends.

Two secondary points, both correct as written:

• Dropping the rb_rc != EOS_ERR_NOT_SUPPORTED tolerance is a genuine tightening and does
  not brick counter-less boards: eos_rollback_verify() (core/rollback.c:129) maps
  EOS_ERR_NOT_SUPPORTED from the HAL to EOS_OK itself. The now-fatal cases are a
  malformed header or a TLV area that does not match its signed hash, which must fail
  closed per .ai/security.md ("Fail closed").
• Verifying but not staging in eos_fw_update_finalize() is right. Staging on install
  would raise the floor for an image that has not booted, removing the fallback — the
  separation include/eos_rollback.h documents.

No layering violation. eos_rollback.h is within eBoot; nothing points up a tier (§5.1).
The TCB does not grow: core/rollback.c was already linked into eboot_core.

Proposed changes

1. Delete eos_image_check_rollback() from core/image_verify.c and its declaration;
   move the demonstration of the old comparison into test_tlv_auth.c under an explicitly
   wrong name. (Finding 1.)
2. Add the eos_fw_update_finalize() regression test described in finding 2. This is the
   one change I would ask for before merge — as it stands the fix is correct but unguarded.
3. Leave finding 3 to a follow-up; folding it in here would mix a structural change with a
   behavioural one, which .ai/architect.md prohibits.

Not checked

• CI: checks.txt is empty. No check runs were reported for this head at bundle time.
  I did not determine whether the workflows had not started, are not required on this
  branch, or did not trigger. Read the finding below as the reason it matters, not as a
  claim about what CI did.

• master does not compile, and this is not caused by this PR. Verified: I extracted
  origin/master (22d8f8b) to a scratch directory and ran the CI's own host
  configuration. It fails with two independent, pre-existing defects:
  include/eos_image.h:135,142 still assert on a reserved member that tlv_len /
  tlv_hash replaced, and core/ed25519_verify.c:338 redefines point_is_identity
  (a merge artifact — the same function appears at :281). The PR head inherits both.
  Consequence for this PR: TCB code is landing with no green build behind it.
  This is already covered by open PRs eBoot#94 and eBoot#98, so per the review policy I
  have opened nothing. #94 or #98 should merge before this.

• I did run this PR's new test, with those two master defects patched out in a scratch
  copy only (git archive to /tmp; the repository was not touched). Result:

  $ cmake -B bld2 -DEBLDR_BUILD_TESTS=ON && cmake --build bld2 --target eboot_test_tlv_auth
  $ ./bld2/tests/eboot_test_tlv_auth
  test_hw_floor_uses_tlv_counter_not_image_version           [PASS]
  8/8 passed          (exit 0)
  

  So the new test does pass and does demonstrate the stated bug. It is not evidence that
  the suite passes on master, because master does not build; the full ctest run and
  the ARM cross build were not run — tests/unit/test_ed25519.c:31,263 has its own
  duplicate-definition merge artifact that blocks the rest of the test target.

• Not verified on hardware. Whether eos_hal_monotonic_read/increment behave as
  assumed on a real OTP-backed board is untested here; the run above uses the simulated
  HAL in test_tlv_auth.c. §28.1 would require a named board before calling T-202
  mitigated on hardware.

• Not examined: whether eos_bootctl_confirm() (core/bootctl.c:159, which calls
  eos_rollback_commit()) executes in the same address space as the eboot_jump_to_app()
  that staged the counter. g_staged_* is file-static RAM state; if confirm runs in the
  application rather than in the bootloader image, the staged value is not visible and the
  floor never advances. That is pre-existing design, not introduced here, but it decides
  whether this fix ends up enforcing anything, and I could not settle it from the tree.

---

Automated architecture review of c9fe84c4b26c — 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#103 "Fix anti-rollback to use authenticated TLV counter"

head: 8c9a363 author: Tanisha1723 ci: none reported

Verdict: Follow-up review (previous head c9fe84c4). The one new commit does exactly
what the last review asked for on findings 1 and 2 and introduces no defect of its own.
Findings 3, 4 and 5 are untouched. What the new test also does, unintentionally, is
document a defect the previous review missed: the update transport cannot deliver the TLV
area at all, so after this PR no image carrying a security counter can be installed.

Previous findings — status

• Finding 1 (delete eos_image_check_rollback()) — resolved in 8c9a363f.
  git grep eos_image_check_rollback 8c9a363f returns nothing. The old comparison now
  lives in tests/unit/test_tlv_auth.c:304 as legacy_broken_version_floor_check(),
  which is the disposition that was recommended.
• Finding 2 (test the changed production paths) — partially addressed. See finding 3
  below: eos_fw_update_finalize() is now driven end-to-end, eboot_jump_to_app() is not.
• Finding 3 (duplicated read → verify → stage sequence) — untouched. Correctly so; the
  previous review deferred it. See finding 4 for why it is worth restating.
• Finding 4 (tests_run hand-maintained literal) — untouched. eBoot#95 is still open.
• Finding 5 (PR body blames MSVC) — untouched. See finding 6.

Findings

#	Severity	File:line	Finding	Recommended fix
1	High	core/fw_update.c:192, root cause core/fw_update.c:139-144	After this PR, no image whose header declares tlv_len > 0 can be installed through eos_fw_update_*. eos_fw_update_write() stops writing once payload_written >= payload_total and moves to EOS_FW_STATE_VERIFY (:139); every later byte falls through both branches, is discarded, still increments total_received (:144) and still returns EOS_OK. The TLV area sits after the payload, so it is exactly the region that gets dropped. eos_fw_update_finalize() then reads it from erased flash and tlv_area_matches_header() fails. Verified: streaming the complete image in 64-byte chunks the way uart_raw_receive() (core/fw_transport_uart.c:112) and xmodem_receive() (:240) do leaves flash@tlv = FF FF FF FF FF FF FF FF FF FF FF FF against an expected 07 69 0C 00 50 00 04 00 09 00 00 00, and finalize returns -2 (EOS_ERR_INVALID) for an image whose counter (9) is at the floor (9) and should install. This is a defect of the PR as a whole, not of the new commit — it was present at c9fe84c4 and the previous review did not catch it. The new test does not catch it either: place_tlv_after_payload() (tests/unit/test_fw_update.c:155) pokes those bytes in with a raw eos_hal_flash_write(), and its own comment records why. Fails closed, so it is a denial of updates rather than a bypass — hence High and not Critical.	Make the write state machine account for the whole container: after the payload completes, enter a TLV state that accepts ctx->header.tlv_len further bytes and writes them at write_addr, only then move to EOS_FW_STATE_VERIFY. finalize() must reject a context whose TLV bytes are short rather than reading whatever is in flash. Independently, eos_fw_update_write() should not return EOS_OK for bytes it discards — a caller that streams a complete artifact and is told every chunk succeeded has no way to learn the artifact was truncated.
2	Medium	core/rollback.c:59-101	The TLV read is bounded against integer overflow but never against the slot. tlv_addr = image_addr + hdr_size + image_size and then tlv_len (up to EOS_TLV_MAX_SIZE = 512) bytes are hashed, with no check that this stays inside the slot. Both callers validate only hdr_size + image_size <= slot_size (stage1/jump_app.c:38-42, core/fw_update.c:90-92). Verified with a probe: a header declaring image_size that fills slot B exactly plus tlv_len = 512 makes eos_rollback_read_image_counter() return EOS_OK with a counter read entirely from outside the slot (slot B = [0x10000..0x18000), tlv area = [0x18000..0x18200), counter = 77). This PR is what makes the path reachable from production for the first time. On the boot path the header is signature-verified first, so this is defence-in-depth; on the install path it is reachable today because finalize() skips signature verification for sig_type < EOS_SIG_ED25519 — which eBoot#104 fixes, so do not duplicate that here.	Add the bound where all three callers get it: reject in eos_rollback_read_image_counter() when the TLV area leaves the slot. That needs the slot limit at hand — either take it as a parameter, or derive it via eos_hal_slot_size() from the slot containing image_addr. Prefer this over patching jump_app.c, which would leave core/secure_boot.c:134 and core/fw_update.c:192 uncovered, and would collide with eBoot#96.
3	Medium	stage1/jump_app.c:57-79	Previous finding 2, still half open. tests/unit/test_fw_update.c now drives eos_fw_update_finalize() to completion and asserts EOS_ERR_ANTI_ROLLBACK, which is what was asked for and which I ran (1/1 pass). The stage-1 hunk still has no test: git grep -l eboot_jump_to_app 8c9a363f -- tests/ is empty. The untested part now includes the new eos_rollback_stage(img_counter) call at :79, which is the only thing that ever raises the hardware floor — if it were dropped the suite would stay green and anti-rollback would silently stop advancing.	Add a test_jump_app case that drives eboot_jump_to_app() with a slot whose TLV counter is below the floor and asserts it returns EOS_ERR_ANTI_ROLLBACK without jumping, plus one above the floor that asserts the staged counter is observable through eos_rollback_commit(). eBoot#96 adds tests/unit/test_jump_app_bounds.c with the HAL-stub scaffolding this needs; rebasing on it once it lands is cheaper than building it twice.
4	Medium	core/secure_boot.c:134,148, stage1/jump_app.c:66,79, core/fw_update.c:192	Previous finding 3, untouched and now larger. The read → verify → stage sequence existed in two copies before this PR; it exists in three after it, the third being the read → verify half in fw_update.c. eos_secure_boot() still has no caller anywhere in stage1/. Restating rather than dropping it because finding 1 is precisely a divergence between two of these copies: the boot path reads a TLV that a factory-flashed image has, and the install path reads one that its own transport never wrote.	Unchanged from the previous review: extract the sequence into one function in core/rollback.c and have all three call sites use it. Still a separate PR — folding it in here would mix a structural change with a behavioural one, which .ai/architect.md prohibits.
5	Low	tests/unit/test_fw_update.c:79,191	Previous finding 4, untouched, and the new file adds a third hand-maintained tests_run literal. Here it is assigned after the tests run, so a forgotten bump reports a spurious failure rather than a silent pass — the safer direction, but still bookkeeping a human has to remember.	No change needed in this PR. eBoot#95 replaces this org-wide and is still open; rebase after it lands.
6	Low	PR body	Previous finding 5, untouched, and the body has now gone stale against its own head. It still attributes the unrunnable suite to "the current MSVC build", which is wrong — origin/master does not compile with gcc either (see Not checked). "## Commit" still lists only 28700bc; the head is 8c9a363f and carries a second commit. The Changes section does not mention the removal of eos_image_check_rollback(), which is an exported symbol disappearing, or the new tests/unit/test_fw_update.c.	Restate the blockage as broken master, tracked by eBoot#94 / #105. Drop the "## Commit" list or regenerate it. Add one line for the removed symbol and one for the new test file.

Architecture conformance

Conforms, on the same reading as the previous review, and nothing in the new commit
changes it.

• §8.1 "Rollback protection where hardware/policy supports it" and .ai/security.md
  ("anti-rollback counters actually checked"): the substitution of the authenticated
  EOS_TLV_MIN_SEC_VER for hdr.image_version is the value §8.1 intends, and
  core/rollback.c:101 binds the TLV area to the signed header before trusting it.
• §5.1: no layering violation. eos_rollback.h is inside eBoot; nothing points up a
  tier. The TCB does not grow — core/rollback.c was already linked into eboot_core,
  and this commit removes 20 lines of TCB code without adding any.
• §21: Tier 1 (Foundation). Boot-time rollback policy belongs in eBoot; correct repo.
• §28: the PR body still states plainly that the suite could not be run rather than
  asserting an unsupported PASS. That remains the right call and is not a finding; only
  the reason it gives is wrong (finding 6).

Finding 1 is a conformance problem in the wider sense — §15's flow is
Download → Verify → Install, and an install that verifies a region the download was
never required to deliver satisfies the diagram while failing every real update. The
design does not currently say otherwise; see the proposal appended for this run.

Proposed changes

1. Fix finding 1 before merge. Everything else in this PR is correct, and it is the one
   item that makes the shipped result worse than what it replaces: today finalize()
   accepts these images (against the wrong counter), after this PR it rejects them
   outright. A merge in this state converts a security hole into a fleet that cannot take
   updates over UART or XMODEM.
2. Fix finding 2 in core/rollback.c, not in the callers.
3. Add the eboot_jump_to_app() test (finding 3), ideally rebased on eBoot#96.
4. Correct the PR body (finding 6). One edit, no code.
5. Leave findings 4 and 5 to eBoot#95 and a separate extraction PR.

I have opened no fix PR. Finding 1 is the only High, and its fix is a state-machine change
in the TCB install path — not the "small and provable" class the review policy permits —
and it cannot be verified as the policy requires anyway, because origin/master does not
build (below), so fix-verify.sh has nothing green to run against.

Not checked

• CI: no checks ran. checks.txt is empty and gh pr checks 103 reports
  no checks reported on the 'fix/eboot-test-issue' branch. This is not a bundle
  artifact — no workflow has run against this head at all. TCB code is being reviewed
  with zero automated evidence behind it. eBoot#90 ("add one job branch protection can
  require") is open and is what would change this.

• origin/master still does not compile, and this PR does not cause it. Verified: I
  extracted 8c9a363f with git archive to a scratch directory (the repository, its
  working tree and its stashes were not touched) and configured the CI's own host build.
  It fails on the two defects the previous review reported, both still present:
  include/eos_image.h:135,142 assert on a reserved member that tlv_len/tlv_hash
  replaced, and core/ed25519_verify.c:338 redefines point_is_identity (already defined
  at :281). eBoot#94 and eBoot#105 both address these and both are still open.
  One of them should merge before this.

• What I did run, in that scratch copy with those two master defects patched out and
  nothing else changed:

  $ cmake -B build/host -DEBLDR_BUILD_TESTS=ON && cmake --build build/host -- -k
  $ ctest --test-dir build/host --output-on-failure --no-tests=error -E ed25519
  100% tests passed, 0 tests failed out of 21
  
  $ ./build/host/tests/eboot_test_fw_update
  test_finalize_rejects_tlv_counter_below_hw_floor           [PASS]   1/1
  $ ./build/host/tests/eboot_test_tlv_auth
  8/8 passed
  

  test_ed25519 is excluded because it cannot compile: tests/unit/test_ed25519.c:31,263
  has its own duplicate-definition merge artifact on master. That is 21 of the 22
  registered tests; the ARM cross build was not run.

• Findings 1 and 2 were each verified with a throwaway probe built in that same
  scratch copy and deleted with it. Neither probe is proposed as a test here — the real
  tests belong with the fixes.

• Not verified on hardware. eos_hal_monotonic_read/increment behaviour on a real
  OTP-backed board is untested; everything above uses simulated flash and a simulated
  counter. §28.1 would require a named board before calling this mitigated on hardware.

• Still not examined, carried forward from the previous review: whether
  eos_bootctl_confirm() (core/bootctl.c, which calls eos_rollback_commit()) executes
  in the same address space as the eboot_jump_to_app() that staged the counter.
  g_staged_counter/g_staged_valid (core/rollback.c:17-18) are file-static RAM; if
  confirm runs in the application image rather than in the bootloader, the staged value is
  invisible there and the floor never advances. Pre-existing design, not introduced here,
  but it decides whether the fix enforces anything, and I still could not settle it from
  the tree. Finding 3's second test case would settle it.

• Not reviewed: the interaction with eBoot#104, which also edits core/fw_update.c
  around finalize(). The two will conflict textually. I checked that #104 does not
  address the TLV area (gh pr diff 104 | grep -c tlv → 0) but did not work out the
  merge order.

---

Automated architecture review of 8c9a363fe4fa — 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#103 "Fix anti-rollback to use authenticated TLV counter"

head: b836304 author: Tanisha1723 ci: none reported

Verdict: Follow-up review (previous head 8c9a363f). The one new commit closes all
three open code findings from the last review — the TLV area is now streamed, the slot
bound landed in core/rollback.c where it was asked for, and eboot_jump_to_app() has
tests — and the PR body is now accurate and its test claims reproduce exactly. What the
same commit introduces is the break the last review's own architecture proposal predicted
and then mis-scoped: it asserted that no in-tree caller streams trailing padding. One
does. xmodem_receive() cannot know the container length, always offers a full 128-byte
block, and is now rejected on the final block of almost every image.

Previous findings — status

• Finding 1 (TLV area never delivered; no image with tlv_len > 0 installable) —
  resolved in b8363040. eos_fw_update_write() gains EOS_FW_STATE_TLV
  (core/fw_update.c:162-186) and finalize() rejects a short tail
  (core/fw_update.c:207). Verified: test_write_streams_tlv_then_finalize_rejects_below_floor
  reaches EOS_FW_STATE_VERIFY with ctx.tlv_written == TLV_AREA_LEN and no raw
  eos_hal_flash_write() poke — the place_tlv_after_payload() helper the last review
  flagged is gone. See finding 1 below for the side effect.
• Finding 2 (TLV read unbounded against the slot) — resolved in b8363040, in the
  recommended place. slot_remain_from() (core/rollback.c:65) derives the limit from
  eos_hal_slot_addr/size() and eos_rollback_read_image_counter() bounds
  hdr_size + image_size + tlv_len against it (core/rollback.c:103-111), so all three
  call sites get it rather than jump_app.c alone. Covered by
  test_tlv_area_must_fit_in_slot (tests/unit/test_tlv_auth.c:293). I re-ran the last
  review's probe scenario through that test: EOS_ERR_INVALID, as required.
• Finding 3 (eboot_jump_to_app() untested) — resolved in b8363040.
  tests/unit/test_jump_app.c adds both cases that were asked for: below-floor returns
  EOS_ERR_ANTI_ROLLBACK with sim_jump_count == 0 and the floor unmoved, above-floor
  stages a counter observable through eos_rollback_commit() (sim_counter 5 → 9). Ran
  it: 2/2. It does not settle the address-space question — see Not checked.
• Finding 4 (read → verify → stage duplicated three ways) — untouched. Still three
  call sites: core/secure_boot.c:134, stage1/jump_app.c:66, core/fw_update.c:252.
  eos_secure_boot() still has no caller in stage1/. Correctly deferred; restated as
  finding 5 only because it stays open.
• Finding 5 (tests_run hand-maintained literal) — untouched, now in four files.
  eBoot#95 is still open. No change wanted here.
• Finding 6 (PR body stale and wrong about MSVC) — resolved. The body now names the
  real blockers (reserved[] assertion, duplicate point_is_identity), the stale
  "## Commit" list is gone, and the removed symbol and new test files are both listed. Its
  five test claims are not just plausible, they reproduce byte for byte — see Not checked.

Findings

#	Severity	File:line	Finding	Recommended fix
1	High	core/fw_update.c:188-192, reached from core/fw_transport_uart.c:240	The new "reject bytes past the container" rule breaks the XMODEM transport for any image whose container size is not a multiple of 128. XMODEM carries no length field, so xmodem_receive() hands eos_fw_update_write() exactly XMODEM_BLOCK_SIZE bytes for every block including the last, which the sender pads. Those pad bytes now land past the container: offset < len at :188 sets EOS_FW_STATE_ERROR and returns EOS_ERR_INVALID, the transport sends CAN and aborts, and finalize() then refuses the poisoned context. Verified by streaming a valid, up-to-date, correctly built image in fixed 128-byte blocks against both trees. At b8363040: container 424 bytes (hdr 156 + payload 256 + tlv 12) → 4 blocks × 128 = 512, 88 pad bytes; blocks 1–3 rc=0, block 4 rc=-2 state=6 (ERROR), finalize=-2. Against origin/master (22d8f8b), the identical stream: all four blocks rc=0, finalize=0. This is a regression this commit introduces, not a pre-existing defect. It fails closed, so it denies updates rather than admitting a bad one — hence High, not Critical. The last review's proposal stated "In-tree there are none — both UART transports stream exactly the length they were given"; that is true of uart_raw_receive() and of ymodem_receive(), which clamps to the size parsed from block 0 (core/fw_transport_uart.c:413-414), and false of XMODEM, which has nothing to clamp to. The same failure reaches YMODEM whenever block 0 carries no parseable size, because file_size is then 0 and the clamp at :413 is skipped. Scope, stated plainly: eos_fw_transport_uart_xmodem() is exported from include/eos_fw_transport.h:90 but has no in-tree caller, so no eBoot flow selects it today — the break is to an integrator wiring up the standard bootloader recovery console, not to a path stage 1 takes.	Block framing is the transport's to strip: XMODEM structurally cannot express an exact length, so the trimming belongs where the container length is known. Give eos_fw_update_write() a companion accessor (eos_fw_update_bytes_wanted(const eos_fw_update_ctx_t *) returning hdr_remaining + payload_remaining + tlv_remaining, 0 once EOS_FW_STATE_VERIFY is reached) and have xmodem_receive() clamp write_len to it exactly as ymodem_receive() already clamps to file_size, ACKing but not forwarding a block once 0 is left. Keep the :188 rejection — it is the right rule for a length-carrying caller. For YMODEM, treat an unparseable block-0 size as a protocol error rather than as "unknown", so it cannot silently fall into the same untrimmed path.
2	Medium	core/fw_transport_uart.c:188-256, tests/unit/test_fw_transport.c	xmodem_receive() has no test at all, which is why finding 1 landed in a PR that added 512 lines of tests and ran five suites green. grep -rn xmodem tests/ matches only crc16_xmodem, a CRC helper the YMODEM cases borrow. The 12 passing cases in eboot_test_fw_transport cover YMODEM (9) and raw (3); the one transport with no length field, and therefore the only one whose framing can disagree with the container, is the one never driven.	Add an XMODEM case to tests/unit/test_fw_transport.c alongside the existing YMODEM ones, using the same scripted-UART harness: script a valid image whose container is deliberately not a multiple of 128, pad the last block, and assert the transfer completes and eos_fw_update_finalize() returns EOS_OK. Written today it fails, which is the point — it is the regression test for finding 1.
3	Medium	core/fw_update.c:65-67 vs :188-192	The same byte stream succeeds or fails depending only on where the caller happens to split its chunks, because over-long input is handled in two places with two outcomes. Verified with a probe at this head: container + 128 trailing bytes in one eos_fw_update_write() call → rc=-2, state ERROR, finalize=-2; the identical bytes split at the container boundary → second call rc=-2 but the entry guard at :65 returns before touching ctx->state, so the context stays in EOS_FW_STATE_VERIFY and finalize returns EOS_OK and installs. A caller cannot tell from the return code which of the two it got, and a transport that retries with different chunking changes the outcome. Fixing finding 1 does not remove this — it only stops XMODEM from being the caller that hits it.	Decide one behaviour and apply it in both places. Rejecting a post-container write without poisoning the context is the more useful of the two (it lets a padded transport be corrected by its caller), so prefer making :188-192 return EOS_ERR_INVALID without setting EOS_FW_STATE_ERROR once tlv_written == tlv_total — but whichever is chosen, both paths must agree, and the choice needs a test that drives the same bytes through both chunkings.
4	Low	include/eos_fw_update.h:36-42, :57-62	Inserting EOS_FW_STATE_TLV = 3 renumbers VERIFY 3→4, COMPLETE 4→5 and ERROR 5→6, and eos_fw_update_ctx_t gains two uint32_t in the middle. Both are in include/, which .ai/architect.md calls "the contract other repos compile against", and eos_fw_update_get_state() returns the renumbered enum. The PR body lists nine behavioural changes and says nothing about either, which is the silence the brief treats as the finding. Bounding it honestly rather than rounding either way: I checked that the enum is not serialised or persisted anywhere — git grep EOS_FW_STATE_ outside core/fw_update.c and the header matches only tests/unit/test_fw_update.c and a stale struct listing in docs/book/book.md:1320 — so no on-flash or on-wire format changes and .efw is untouched. The exposure is source and link level only, to an out-of-tree consumer that does not recompile.	One line in the PR body under Changes: the state enum is renumbered and the context struct grows, both require recompilation of any out-of-tree consumer, neither affects the image format or any persisted structure. No code change.
5	Low	core/secure_boot.c:134, stage1/jump_app.c:66, core/fw_update.c:252	Previous finding 4, untouched and still open. Restating only so it is not dropped: three copies of read → verify → stage, and finding 1 is again a divergence between two receivers of the same container — the boot path reads a TLV that a factory-flashed image has, and the XMODEM install path now cannot deliver one.	Unchanged: extract the sequence into one function in core/rollback.c and have all three call it. Still a separate PR — folding a structural change into a behavioural one is what .ai/architect.md prohibits.
6	Low	tests/unit/test_fw_update.c:230, tests/unit/test_jump_app.c:264, tests/unit/test_tlv_auth.c:364	Previous finding 5, untouched, now four hand-maintained tests_run literals across the suite. All three new ones assign after the run, so a forgotten bump reports a spurious failure rather than a silent pass — the safe direction.	No change here. eBoot#95 replaces this org-wide and is still open; rebase after it lands.
7	Low	docs/book/book.md ch. 16 "Stream-Based Update Pipeline"	The chapter describes the update pipeline as "processes data in 256-byte chunks" with no mention of a TLV tail, and this PR makes the tail mandatory and makes over-long input an error. The struct it prints at :1320 was already wrong on master (none of its fields match eos_fw_update_ctx_t), so this PR does not newly break that part — but the described contract is newly incomplete, and per project rules a change that makes existing documentation wrong is not finished. The header doc comment in include/eos_fw_update.h:84-88 was updated correctly, so this is the book only.	Two sentences in ch. 16: the container is [header][payload][TLV area] and eos_fw_update_write() must be fed all three; bytes past the container are rejected, not discarded. Regenerating or deleting the stale struct listing at :1320 is a separate pre-existing cleanup.

Architecture conformance

Conforms, on the same reading as both previous reviews, and the new commit strengthens
two of the points rather than changing them.

• §8.1 "Rollback protection where hardware/policy supports it" and .ai/security.md
  ("anti-rollback counters actually checked"): the authenticated EOS_TLV_MIN_SEC_VER
  now actually arrives on the install path instead of being read from erased flash, which
  is what §8.1 intends and what the previous head could not deliver.
• .ai/security.md "Fail closed": honoured throughout the new commit. A truncated
  TLV tail leaves the context in EOS_FW_STATE_TLV, and finalize() requires
  EOS_FW_STATE_VERIFY and re-checks tlv_written == header.tlv_len at
  core/fw_update.c:207, so a forced context cannot hash a partial region. The new
  EOS_FW_STATE_IDLE guard at :66 also closes a smaller hole: a write before
  eos_fw_update_begin() previously fell through every branch and returned EOS_OK.
  Finding 1 is a fail-closed defect too — it denies updates, it does not admit one.
• §5.1: no layering violation. slot_remain_from() reaches down to
  eos_hal_slot_addr/size(), which is the correct direction; nothing points up a tier.
  The TCB grows by ~44 lines of bounds checking in core/rollback.c, which is the kind
  of growth "minimal and auditable" is for.
• §21: Tier 1 (Foundation). Boot-time and install-time rollback policy belongs in
  eBoot; correct repo.
• §28: the body's five test claims are backed, and I reproduced all five exactly (Not
  checked). This is the first head of this PR where the Testing section is evidence rather
  than a statement of blockage, and the remaining blockage is still stated plainly.

Finding 1 is also a conformance gap in the wider sense, and it is a different one from
the gap the last review filed. That proposal asked what an install must consist of; this
head implements it and then runs into who is responsible for stripping a transport's own
framing when the transport cannot express the container's length. An addendum is appended
for this run.

Proposed changes

1. Fix finding 1 before merge, in xmodem_receive() and not in eos_fw_update_write().
   The :188 rejection is correct and is the whole point of the commit; what is missing
   is the length XMODEM never had. This is the one item that leaves the shipped result
   worse than what it replaces on that transport.
2. Add the XMODEM transport test (finding 2) in the same PR as the fix — it is the
   regression test, and without it the next framing change repeats this.
3. Resolve the chunk-boundary asymmetry (finding 3) while the state machine is open.
4. Add one line to the PR body for the enum and struct change (finding 4) and two
   sentences to book ch. 16 (finding 7). No code.
5. Leave findings 5 and 6 to eBoot#95 and a separate extraction PR.
6. Merge order, unchanged and now more pressing: eBoot#94 or #105 first — this head
   still does not build on its own (Not checked). Note also that this PR now bounds the
   TLV area inside eos_rollback_read_image_counter(), which overlaps eBoot#96
   ("harden application slot boundary validation"), and still conflicts textually with
   eBoot#104 around finalize().

I have opened no fix PR. Finding 1 is the only High, and its fix needs a new accessor on a
public header in the TCB plus a transport change — a design decision, not the "small and
provable" class the review policy permits. It could not be verified as the policy requires
in any case: fix-verify.sh has nothing green to run against while master does not
build. I checked the 20 open PRs and none touches core/fw_transport_uart.c or the XMODEM
path, so nothing is duplicated by saying it here instead.

Not checked

• CI: no checks ran, again. checks.txt is empty and gh pr checks 103 reports
  no checks reported on the 'fix/eboot-test-issue' branch. Three heads of TCB code with
  zero automated evidence. mergeStateStatus is BLOCKED and reviewDecision is
  REVIEW_REQUIRED; mergeable is MERGEABLE, so the block is review, not conflict.
  eBoot#90 is what would change this and is still open.

• This head still does not compile as-is, and this PR does not cause it. Verified: I
  extracted b8363040 with git archive to a scratch directory — the repository, its
  working tree, its branch (fix/ed25519-low-order-keys) and its stashes were not touched
  — and configured the CI's own host build. Both defects the previous two reviews reported
  are still present at this head: include/eos_image.h:135,142 assert on a reserved
  member that tlv_len/tlv_hash replaced, and core/ed25519_verify.c:338 redefines
  point_is_identity (already at :281). eBoot#94 and eBoot#105 both address these and
  both are open.

• What I did run, in that scratch copy with those two master defects patched out and
  nothing else changed:

  $ cmake -B build/host -DEBLDR_BUILD_TESTS=ON && cmake --build build/host --parallel 4 -- -k
  $ ctest --test-dir build/host --output-on-failure --no-tests=error -E ed25519
  100% tests passed, 0 tests failed out of 22
  
  $ ./build/host/tests/eboot_test_fw_update      4/4 passed
  $ ./build/host/tests/eboot_test_tlv_auth       9/9 passed
  $ ./build/host/tests/eboot_test_jump_app       2/2 passed
  $ ./build/host/tests/eboot_test_fw_transport   12/12 passed
  $ ./build/host/tests/eboot_test_rollback       14/14 passed
  

  Every number the PR body claims reproduces exactly. That is 22 of the 23 registered
  tests; test_ed25519 is excluded because it cannot compile —
  tests/unit/test_ed25519.c:31,263 carries its own duplicate-definition merge artifact
  on master. The ARM cross build was not run.

• Findings 1 and 3 were each verified with a throwaway probe built in that scratch copy
  and in a second scratch copy of origin/master (22d8f8b), both deleted afterwards.
  Neither probe is offered as a test here; finding 2 describes where the real one belongs.

• git diff --check — NOT RUN. The body claims it passed; I did not reproduce that one.

• Not verified on hardware. eos_hal_monotonic_read/increment on a real OTP-backed
  board is still untested — everything above is simulated flash and a simulated counter.
  §28.1 would require a named board before calling this mitigated on hardware.

• test_jump_app.c does not settle the carried-forward address-space question, and I
  still cannot. The last review hoped finding 3's second test case would decide whether
  eos_bootctl_confirm() (core/bootctl.c, which calls eos_rollback_commit()) runs in
  the same address space as the eboot_jump_to_app() that staged the counter, because
  g_staged_counter/g_staged_valid (core/rollback.c:17-18) are file-static RAM. It
  cannot: test_jump_stages_tlv_counter_above_floor calls eos_rollback_commit() in the
  same process as the jump, which assumes the answer rather than establishing it. If
  confirm executes in the application image, the staged value is invisible there and the
  floor never advances. Pre-existing design, not introduced here, but it still decides
  whether any of this enforces anything.

• slot_remain_from() skips a slot whose base address is 0 (core/rollback.c:79),
  which would make eos_rollback_read_image_counter() return EOS_ERR_INVALID for an
  image in it — fail-closed, but a boot failure. Not raised as a finding because I checked
  all 83 board ports under boards/ and none places slot_a_addr at 0; it is latent, not
  live. Worth knowing before a board port does.

• The two .c files under poc/ were not reviewed — poc/tlv_downgrade_poc.c calls
  eos_rollback_read_image_counter() and is unaffected by this diff, but I did not check
  whether it still builds or still demonstrates what it claims.

• Not reviewed: the merge order against eBoot#104, which also edits core/fw_update.c
  around finalize(), and against eBoot#96, which now overlaps this PR's slot bound. Both
  will conflict textually with this head; I did not work out the resolution.

---

Automated architecture review of b83630401f21 — 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#103 "Fix anti-rollback to use authenticated TLV counter"

head: 984a0d3 author: Tanisha1723 ci: none reported

Verdict: Follow-up review (previous head b8363040). The one new commit closes the
last review's High finding in the place it was asked for — xmodem_receive() now clamps to
a new eos_fw_update_bytes_wanted() accessor instead of pushing its own block padding into
the container — and closes findings 2, 3, 4 and 7 as well. I reproduced all five of the
body's test claims exactly, and additionally ran the ARM cross build, which no previous
review of this PR ran: it passes. What the same commit leaves is the mirror image of the
finding it fixes: YMODEM was hardened against a missing block-0 size but was not given the
container-aware clamp XMODEM got, so it still clamps to the sender's declared file size
and now aborts a transfer that completed on master. Separately, and larger than this PR:
no in-tree tool can produce an image carrying the TLV counter this PR makes authoritative.

Previous findings — status

• Finding 1 (XMODEM final-block padding rejected; container-not-multiple-of-128
  uninstallable) — resolved in 984a0d39. xmodem_receive() (core/fw_transport_uart.c:244-262)
  loops on eos_fw_update_bytes_wanted() (core/fw_update.c:303) and forwards only what
  the container still wants. Verified beyond the shipped test: I streamed a TLV-carrying
  424-byte container (hdr 156 + payload 256 + TLV 12, so block 4 carries 88 pad bytes and
  the payload→TLV transition) in 128-byte XMODEM blocks through the real ops table —
  rc=0, state VERIFY, tlv_written 12/12, flash byte-identical to the container,
  finalize=0. The equivalent stream returned -2 at b8363040.
• Finding 2 (xmodem_receive() had no test at all) — resolved.
  test_xmodem_padded_final_block_completes_and_finalizes (tests/unit/test_fw_transport.c:565)
  drives the ops table through the scripted-UART harness. Ran it: 13/13. One gap in what it
  covers — see finding 4 below.
• Finding 3 (same bytes succeed or fail depending on chunking) — partially addressed.
  The state-poisoning half is fixed: core/fw_update.c:193 no longer sets
  EOS_FW_STATE_ERROR, so both the entry guard (:65-67) and the trailing check agree —
  return EOS_ERR_INVALID, leave the context in VERIFY. test_trailing_byte_is_rejected_the_same_across_chunk_boundaries
  asserts equality of the return code, the state and tlv_written, and it passes. It stops
  one field short of the claim in its name; see finding 5.
• Finding 4 (enum renumbering and struct growth undocumented) — resolved. The body now
  carries a Compatibility section naming the EOS_FW_STATE_TLV = 3 renumbering, the two
  new uint32_t, and the recompilation requirement. Its assertion that no persisted or
  on-wire format changes is correct — I re-checked git grep EOS_FW_STATE_ at this head:
  outside core/fw_update.c and the header it matches only the unit tests and the stale
  struct listing in docs/book/book.md.
• Finding 5 (read → verify → stage duplicated three ways) — untouched. Still
  core/secure_boot.c:134, stage1/jump_app.c:66, core/fw_update.c:253, and
  eos_secure_boot() still has no caller in stage1/. Correctly deferred; restated as
  finding 7 only so it is not dropped.
• Finding 6 (tests_run hand-maintained literals) — untouched, now four of them
  (test_fw_transport.c:669 → 13, test_fw_update.c:300 → 6, test_tlv_auth.c:364 → 9,
  test_jump_app.c:268 → 2). eBoot#95 is still open. No change wanted here.
• Finding 7 (book ch. 16 made incomplete) — resolved. docs/book/book.md:1324 adds the
  two sentences that were asked for, verbatim in substance. The pre-existing stale struct
  listing at :1320 is untouched, which is correct — it was already wrong on master.

Findings

#	Severity	File:line	Finding	Recommended fix
1	Medium	core/fw_transport_uart.c:437-441	YMODEM did not get the fix XMODEM got, and a transfer that completed on master now aborts. The write clamp is still write_len = file_size - total_received — the size the sender declared for the file, not the length of the container the header describes. When the file is longer than hdr_size + image_size + tlv_len, the surplus reaches eos_fw_update_write(), hits the offset < len rejection at core/fw_update.c:193, and ymodem_receive() sends CAN and returns. Verified by running the identical scripted transfer against both trees with a tlv_len = 0 image so the only difference is the trailing-byte rule: declared size 512 over a 384-byte container gives rc=0, can=0 at origin/master (22d8f8b) and rc=-2, can=1 at 984a0d39. The image was fully and correctly written to slot B in both cases — the transfer is failed after the bytes are already there, so this is denial of updates, not a bad install, hence Medium and not Critical. Narrower than the XMODEM break the last review filed, because the in-tree packers emit a file whose length equals the container exactly (tools/imgpack.py, tools/eos_sign.py), so the golden path is unaffected; it bites a sender that reports an on-disk size for an artifact with any trailing or padded region, which is the ordinary output of a packaging step that aligns to a flash page. eos_fw_transport_uart_ymodem() is exported from include/eos_fw_transport.h and has no in-tree caller, the same scope the XMODEM finding had.	Use the same accessor the XMODEM path now uses. Replace the file_size clamp with eos_fw_update_bytes_wanted(ctx), keeping file_size only as the protocol-level end-of-file signal it already is at :456. That makes the container the single authority on how many bytes are image data, on both block transports, and removes the disagreement between them.
2	Medium	core/fw_transport_uart.c:244-262, verified via probe	XMODEM silently accepts and discards whole data blocks once the container is complete, which is exactly the behaviour the commit changed core/fw_update.c to stop doing, reinstated one layer up. Once eos_fw_update_bytes_wanted() returns 0 the while breaks immediately, the block is ACKed, expected_blk advances, and the loop reads the next one. Verified: appending one full 128-byte data block of 0xDE after a complete 424-byte container gives rc=0, no NAK, no CAN, and finalize=0 — an entire extra block, not padding, absorbed and reported as a successful transfer. The installed bytes are only ever the container and are hash-verified, so nothing bad is admitted; what is lost is any ability to notice that the sender and the receiver disagree about which artifact was sent. The PR body states "Reject bytes beyond the complete container rather than silently discarding them", which is true of eos_fw_update_write() and not true of the transport that calls it. Untested in either direction.	XMODEM padding is bounded by construction: it can only ever be the tail of the block in which the container ends. Track that — once wanted reaches 0, tolerate the remainder of the current block and treat a subsequent SOH data block as a protocol error (CAN, EOS_ERR_INVALID), rather than tolerating an unbounded tail. Add the case to tests/unit/test_fw_transport.c next to the padded-block test.
3	Medium	tools/imgpack.py:74, tools/eos_sign.py:105	Nothing in the repository can produce an image carrying the counter this PR makes authoritative. Both packers write b'\x00' * 30 for tlv_len(2) + tlv_hash(28) unconditionally, with a comment saying 0 = no authenticated TLV area; tests/production_test_suite.py:150 does the same. grep -rl EOS_TLV_INFO_MAGIC over .c/.h outside tests/ and poc/ returns only core/image_tlv.c and include/eos_image_tlv.h — the reader, never a writer. So every artifact the tree can build has tlv_len == 0, eos_rollback_read_image_counter() returns counter 0 by the documented conservative path (core/rollback.c:129), and the anti-rollback floor this PR wires into both the boot and the install path can never be raised by an in-tree image. The behaviour is coherent and fail-closed — a 0 counter can only fail against a floor, never advance one — so this is not a bypass and not a defect introduced here. It is why the feature cannot reach §28 Validated however many unit tests pass: the consumer is complete and has no producer. Raised on this PR because this is the PR that makes the counter load-bearing.	Out of scope for this PR — say so in the body under Scope, next to the existing note about master and the duplicated boot logic, so the gap is recorded rather than implied closed by the passing tests. The work itself is a --min-sec-ver N option on tools/imgpack.py and tools/eos_sign.py that emits an EOS_TLV_MIN_SEC_VER entry and sets tlv_len/tlv_hash over it, plus a round-trip test that packs an image and drives it through eos_rollback_read_image_counter(). That is its own PR, and it is the prerequisite for calling T-202 mitigated on anything.
4	Medium	tests/fuzz/fuzz_fw_update.c:18-20	.ai/security.md requires fuzz coverage, not just unit tests, for every externally reachable parser, and eos_fw_update_write() is one — this PR rewrites its state machine and adds a new region to it. The harness that names it fuzzes an API that does not exist: it declares extern int eos_fw_update_init(void), eos_fw_update_process_chunk(const uint8_t*, size_t) and eos_fw_update_finalize(void), none of which are the real signatures in include/eos_fw_update.h. So the container parser has no fuzz coverage in fact, only in the file name. Not a finding against this PR and no fix is offered here: eBoot#99 ("point every harness at the API it claims to fuzz") and eBoot#101 ("compile the harnesses, and refuse the extern that hid the bugs") are both open and both cover exactly this. Recorded so the new streaming path is re-fuzzed once one of them lands.	Nothing in this PR. After #99 or #101 merges, add a fuzz_fw_update corpus entry that walks a container across chunk boundaries — header split, payload→TLV straddle, short TLV tail, trailing bytes — since those are the transitions this PR introduced.
5	Low	core/fw_update.c:193 vs :196, tests/unit/test_fw_update.c:245	Previous finding 3 is not quite closed. eos_fw_update_write() returns before ctx->total_received += len, so the container bytes consumed in a call that also carried a trailing byte are never counted. Verified: identical bytes, container + one trailing byte, single (one call) → total_received = 0, eos_fw_update_progress() = 0%; split (two calls) → total_received = 424, progress = 100%. eos_fw_update_progress() is public API in include/eos_fw_update.h, so a transport reporting progress shows 0% for a container it received in full. test_trailing_byte_is_rejected_the_same_across_chunk_boundaries asserts equality of the return code, the state and tlv_written, and stops before the one field that still differs — so the test named for this property passes while the property does not fully hold.	Count what was actually consumed rather than what was offered: ctx->total_received += (uint32_t)offset; before the offset < len return, and drop the separate += len on the success path (they are the same value when offset == len). Then extend the existing test with ASSERT(single.total_received == split.total_received);.
6	Low	tests/unit/test_fw_transport.c:521-545	The XMODEM regression test builds its image with hdr.tlv_len = 0 (:540) and flags = 0, so the one transport test for the padded final block never drives EOS_FW_STATE_TLV — the only state this PR series added — and never exercises a payload→TLV transition inside a padded block, which is the case the whole PR exists for. The transitions are covered in isolation by test_bytes_wanted_tracks_the_remaining_container, but only at exact chunk boundaries chosen to align with them, never straddling. I confirmed by probe that the real combination works, so this is missing coverage rather than a defect.	Give build_unaligned_image() a TLV area: 156 + 256 + 12 = 424 bytes is already the length the test uses and already splits the payload→TLV boundary inside block 4. tests/unit/test_fw_update.c:140-155 has the eight lines of TLV construction to lift.
7	Low	core/secure_boot.c:134, stage1/jump_app.c:66, core/fw_update.c:253	Previous finding 5, untouched and still open. Restating only so it is not dropped: three copies of read → verify → stage, and findings 1 and 2 are once again a divergence between two receivers of the same container — XMODEM was taught the container's length this commit and YMODEM was not.	Unchanged: extract the sequence into one function in core/rollback.c and have all three call it. Still a separate PR — folding a structural change into a behavioural one is what .ai/architect.md prohibits.
8	Low	tests/unit/test_fw_update.c:300, test_fw_transport.c:669, test_jump_app.c:268, test_tlv_auth.c:364	Previous finding 6, untouched. Four hand-maintained tests_run literals; all four assign after the run, so a forgotten bump reports a spurious failure rather than a silent pass — the safe direction.	No change here. eBoot#95 replaces this org-wide and is still open; rebase after it lands.

Architecture conformance

Conforms, on the same reading as all three previous reviews. The new commit does not move
any boundary.

• §8.1 "Rollback protection where hardware/policy supports it" and .ai/security.md
  ("anti-rollback counters actually checked"): unchanged by this commit and still correct —
  the authenticated EOS_TLV_MIN_SEC_VER is what both the boot path and the install path
  read, bound to the signed header through tlv_len/tlv_hash. Finding 3 is the honest
  qualifier on that: the check is right and nothing in the tree can yet feed it a counter.
• .ai/security.md "Fail closed": honoured. Both new rejections fail closed — YMODEM
  on an unusable block-0 size (core/fw_transport_uart.c:407) and the container rule at
  core/fw_update.c:193. Finding 1 is a fail-closed defect too: it denies an update, it
  does not admit one. The one place worth naming explicitly is the removal of
  ctx->state = EOS_FW_STATE_ERROR from the trailing-byte path: a context that saw
  trailing bytes now stays installable, and eos_fw_update_write() reports the error only
  through its return value. That is what the last review recommended and it is safe on
  inspection — the surplus bytes are never written to flash and never enter the SHA/CRC, so
  what finalize() installs is exactly the container it verified — but it does move the
  burden onto the caller to check a return code. All three in-tree transports do. Recorded
  rather than filed as a finding.
• §5.1: no layering violation. eos_fw_update_bytes_wanted() is a read-only accessor
  over the caller's own context; the transport calls down into the update core, which is
  the correct direction, and nothing points up a tier. TCB growth is ~20 lines of accessor
  plus a clamp loop.
• §21: Tier 1 (Foundation). Boot-time and install-time rollback policy, and the update
  transports, belong in eBoot; correct repo.
• §28: the body's five test claims are backed and all five reproduce exactly (Not
  checked). The Scope section states plainly what the PR does not address. The one thing
  the body asserts that does not hold is the transport-level half of "bytes beyond the
  complete container are rejected rather than discarded" — finding 2.

Finding 3 is a conformance gap of the design's own, not of this PR: §8.1 requires rollback
protection to be checked and §15.1 requires signed metadata, and no section makes anyone
responsible for emitting the anti-rollback counter. A proposal is appended for this run.
It is distinct from the two container/transport proposals already filed against this PR —
those ask what an install must consist of and who strips a transport's framing; this one
asks who writes the field in the first place.

Proposed changes

1. Fix finding 1 in ymodem_receive() with the accessor xmodem_receive() already uses.
   It is a two-line change and it is the only item that leaves a shipped result worse than
   master on a transport.
2. Bound the XMODEM tail (finding 2) and add the block-after-container test, in the same
   PR as 1 — both are the same question about who owns block framing.
3. Extend the padded-block test to carry a TLV area (finding 6) and add the
   total_received assertion (finding 5). Small, and they close the two claims that
   currently overstate slightly.
4. Add one line to Scope for the missing packer support (finding 3). No code here; the
   packer work is its own PR and is the prerequisite for any hardware validation.
5. Leave findings 4, 7 and 8 to eBoot#99/#101, a separate extraction PR, and eBoot#95.
6. Merge order, unchanged and now the last thing standing: eBoot#94 or #105 first —
   this head still does not build on its own (Not checked). This PR still overlaps eBoot#96
   on the slot bound in eos_rollback_read_image_counter() and still conflicts textually
   with eBoot#104 around finalize(); I confirmed #104 touches core/fw_update.c and does
   not touch core/fw_transport_uart.c.

I have opened no fix PR. Nothing at this head is rated High, and the review policy permits
fix PRs only for High findings that are small and provable; finding 1 is provable and
small, but it is Medium, and fix-verify.sh has nothing green to run against in any case
while master does not build. I checked all 20 open PRs: none touches
core/fw_transport_uart.c and none adds TLV emission to tools/imgpack.py or
tools/eos_sign.py (eBoot#107 edits imgpack.py for version-component overflow only), so
nothing here duplicates existing work.

Not checked

• CI: no checks ran, for the fourth head running. checks.txt is empty,
  gh pr checks 103 reports no checks reported on the 'fix/eboot-test-issue' branch, and
  statusCheckRollup is []. mergeStateStatus is BLOCKED with mergeable: MERGEABLE
  and reviewDecision: REVIEW_REQUIRED, so the block is review, not conflict. Four heads of
  TCB code with zero automated evidence behind them. eBoot#90 is what would change this and
  is still open.

• This head does not compile as-is, and this PR does not cause it. Verified afresh
  rather than inherited: I extracted 984a0d39 with git archive to a scratch directory
  — the repository, its working tree, its branch (fix/ed25519-low-order-keys) and its
  stashes were not touched — and ran the CI's own host configuration unmodified. The first
  translation unit fails: include/eos_image.h:135,142 assert on a reserved member that
  tlv_len/tlv_hash replaced; 1 target of 40-odd builds. The duplicate
  point_is_identity at core/ed25519_verify.c:281 and :338 is present too. eBoot#94 and
  eBoot#105 both fix these and both are open.

• What I did run, in a second scratch copy with only those two master defects patched
  out and nothing else changed:

  $ cmake -B build/host -DEBLDR_BUILD_TESTS=ON && cmake --build build/host --parallel 4 -- -k
  $ ctest --test-dir build/host --output-on-failure --no-tests=error -E ed25519
  100% tests passed, 0 tests failed out of 22
  
  $ ./build/host/tests/eboot_test_fw_update      6/6 passed
  $ ./build/host/tests/eboot_test_tlv_auth       9/9 passed
  $ ./build/host/tests/eboot_test_jump_app       2/2 passed
  $ ./build/host/tests/eboot_test_fw_transport   13/13 passed
  $ ./build/host/tests/eboot_test_rollback       14/14 passed
  

  Every number the PR body claims reproduces exactly. That is 22 of the 23 registered
  tests; test_ed25519 is excluded because it cannot compile —
  tests/unit/test_ed25519.c:31,263 carries its own duplicate-definition merge artifact on
  master, independent of the two above.

• The ARM cross build PASSES, and this is the first review of this PR to run it. Same
  patched scratch copy, the exact configuration from .github/workflows/build.yml:44-52:

  $ cmake -B build-arm -DCMAKE_SYSTEM_NAME=Generic -DCMAKE_C_COMPILER=arm-none-eabi-gcc \
      -DEBLDR_BOARD=stm32f4 \
      -DCMAKE_C_FLAGS="-mcpu=cortex-m4 -mthumb -mfloat-abi=hard -mfpu=fpv4-sp-d16 -specs=nosys.specs"
  $ cmake --build build-arm --parallel 4          # exit 0, ebldr_stage0.elf linked
  

  No warning or error in any file this PR touches, on either the host or the ARM build. The
  warnings that do appear (core/power_init.c:84, core/ecc_scrub.c:65,
  core/bmc_handoff.c:55 -Wformat on uint32_t under a 32-bit long;
  core/ed25519_verify.c:313 unused key_has_prime_order) are all pre-existing and in
  untouched files.

• git diff --check — PASS. git diff --check 22d8f8b..984a0d39 is clean. The body
  claims this and it is the one claim the last review did not reproduce; it holds.

• Findings 1, 2 and 5 were each verified with a throwaway probe built in that scratch
  copy, and finding 1 additionally in a second scratch copy of origin/master (22d8f8b) so
  the two trees ran the identical scripted transfer. Both scratch copies were deleted
  afterwards. None of the probes is offered as a test here; findings 2, 5 and 6 say where
  the real ones belong.

• Not verified on hardware. eos_hal_monotonic_read/increment on a real OTP-backed
  board is still untested — everything above is simulated flash and a simulated counter.
  §28.1 would require a named board, and finding 3 says there is not yet an artifact to put
  on one.

• The address-space question is still unsettled and I still cannot settle it. Carried
  forward from all three previous reviews: whether eos_bootctl_confirm()
  (core/bootctl.c, which calls eos_rollback_commit()) executes in the same address space
  as the eboot_jump_to_app() that staged the counter. g_staged_counter/g_staged_valid
  (core/rollback.c:17-18) are file-static RAM; test_jump_stages_tlv_counter_above_floor
  calls commit in the same process as the jump, which assumes the answer. If confirm runs in
  the application image the staged value is invisible there and the floor never advances.
  Pre-existing design, not introduced here, but together with finding 3 it means neither end
  of the anti-rollback mechanism has been shown to work outside a unit test.

• slot_remain_from() still skips a slot based at address 0 (core/rollback.c:79).
  Unchanged from the last review, still latent — I re-checked and none of the 83 board ports
  under boards/ places slot_a_addr at 0.

• The two .c files under poc/ were not reviewed. poc/tlv_downgrade_poc.c calls
  eos_rollback_read_image_counter() and is untouched by this diff; I did not check whether
  it still builds or still demonstrates what it claims.

• Merge order against eBoot#96 and eBoot#104 was not worked out. Both will conflict
  textually with this head — #96 on the slot bound, #104 on finalize(). I confirmed the
  overlap and did not resolve it.

• existing-comments.txt is empty, so nothing here repeats a human reviewer; there are
  none on this PR.

---

Automated architecture review of 984a0d39a8e8 — 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#103 "Fix anti-rollback to use authenticated TLV counter"

head: 180680c author: Tanisha1723 ci: none reported

Verdict: Follow-up review (previous head 984a0d39). The one new commit closes the two
transport findings from the last review in the places they were asked for — YMODEM now
clamps to the container instead of the sender's declared file size, XMODEM refuses a whole
data block that arrives after the container is complete — plus the total_received
accounting and the TLV-less padded-block test. I reproduced every claim in the body and ran
the ARM cross build; both pass. What the commit does not do is give YMODEM the guard it
gave XMODEM: the shared helper stops offering bytes but the YMODEM loop keeps ACKing whole
blocks past the container and reports the transfer as successful. That is the same finding
the last review filed against XMODEM, now surviving in the other transport — the third
consecutive head where a fix landed on one of the two block transports and not the other.

Previous findings — status

• Finding 1 (YMODEM clamped to file_size, aborting a transfer that completed on
  master) — resolved in 180680cc. ymodem_receive() now computes file_len as the
  YMODEM framing bound only (core/fw_transport_uart.c:472-475) and hands it to the new
  shared write_container_bytes() (:203-222, called at :492), which stops at
  eos_fw_update_bytes_wanted(). Verified beyond the shipped test: a declared file size of
  896 over a 424-byte TLV-carrying container gives rc=0, no CAN, tlv_written 12/12,
  flash byte-identical to the container, finalize=0. The recommended fix was taken
  verbatim, including keeping file_size as the end-of-file signal.
• Finding 2 (XMODEM silently accepted and discarded whole blocks past the container) —
  resolved for XMODEM, still open for YMODEM. core/fw_transport_uart.c:275-284 now
  sends CAN and returns EOS_ERR_INVALID when a correctly sequenced block arrives with
  bytes_wanted() == 0, bounding the tolerated surplus to the remainder of the block the
  container ends in, exactly as recommended.
  test_xmodem_data_block_after_container_is_rejected (tests/unit/test_fw_transport.c)
  covers it; I ran it. The same shape over YMODEM is finding 1 below.
• Finding 3 (nothing in the repo can emit the counter this PR makes authoritative) —
  untouched. Re-checked at this head: tools/imgpack.py:74 and tools/eos_sign.py:105
  still write b'\x00' * 30 for tlv_len(2) + tlv_hash(28), and
  git grep -l EOS_TLV_INFO_MAGIC 180680cc -- '*.c' '*.h' '*.py' outside tests/ and
  poc/ still returns only the reader (core/image_tlv.c, include/eos_image_tlv.h). The
  one-line Scope addition the last review asked for was not made. Restated as finding 5.
• Finding 4 (fuzz harness fuzzes an API that does not exist) — untouched, correctly:
  eBoot#99 and eBoot#101 both cover it and both are still open. Not a finding against this
  PR. Recorded again only so the new streaming path is re-fuzzed once one of them lands.
• Finding 5 (total_received not counted on a call that also carried a trailing byte) —
  resolved in 180680cc. core/fw_update.c:198 now adds offset before the
  offset < len return and the separate += len is gone, which is the recommended fix
  exactly. test_trailing_byte_is_rejected_the_same_across_chunk_boundaries gained the
  three assertions that were asked for, including
  eos_fw_update_progress(&single) == eos_fw_update_progress(&split). Verified
  independently: a 3-block partial transfer now reports total_received = 384,
  progress = 93, bytes_wanted = 40, all consistent.
• Finding 6 (padded-block test never drove EOS_FW_STATE_TLV) — resolved.
  build_unaligned_image() was replaced by build_container(), which builds
  156 + 256 + 12 = 424 bytes with a real [tlv_info][entry_hdr][uint32] area authenticated
  by hdr.tlv_hash, so block 4 now carries the payload→TLV transition, the whole TLV area
  and 88 bytes of padding. Both block-transport tests assert ctx.tlv_written == 12.
• Finding 7 (read → verify → stage duplicated three ways) — untouched. Still
  core/secure_boot.c:134, stage1/jump_app.c:66, core/fw_update.c:259. Correctly
  deferred; restated as finding 6 only so it is not dropped.
• Finding 8 (tests_run hand-maintained literals) — untouched, still four
  (test_fw_transport.c:768 → 15, test_fw_update.c:307 → 6, test_tlv_auth.c:364 → 9,
  test_jump_app.c:268 → 2). eBoot#95 is still open. No change wanted here.

Findings

#	Severity	File:line	Finding	Recommended fix
1	Medium	core/fw_transport_uart.c:487-502	YMODEM did not get the guard XMODEM got this commit, so it still accepts and discards whole data blocks once the container is complete. write_container_bytes() breaks as soon as eos_fw_update_bytes_wanted() returns 0 and returns EOS_OK (:210, :221); ymodem_receive() then adds the whole block to total_received (:499), ACKs it (:500) and reads the next one. The only thing that ever stops this is file_len == 0 at :481 — the sender's own declared size, parsed from block 0 and bounded only by UINT32_MAX at :427. Verified by probe against this head: three full 128-byte blocks of 0xDE-style filler after a complete 424-byte container, declared size 896, give rc=0, no CAN, tlv_written 12/12, finalize=0 — 384 unaccounted bytes absorbed and the install allowed to proceed. The identical block sequence over XMODEM gives rc=-2 and CAN. Nothing bad is installed (flash is byte-identical to the container and hash-verified), so this is loss of detection, not a bypass — hence Medium, the same rating the last review gave the XMODEM half. The PR body's own justification for the XMODEM guard ("accepting them would accept an unbounded amount of unaccounted data", :277-279) applies unchanged here: file_size is asserted by the same sender whose bytes are in question, so it is not an independent bound.	Mirror the XMODEM guard. After the retransmit and sequence checks and before write_container_bytes(), reject a block that begins with the container already complete: if (eos_fw_update_bytes_wanted(ctx) == 0) { c = XMODEM_CAN; eos_hal_uart_send(&c, 1); return EOS_ERR_INVALID; }. Padding inside the block the container ends in is still consumed by the existing clamp, which is the distinction the XMODEM comment already draws. Add the case next to test_ymodem_declared_size_larger_than_container_is_clamped, which today covers only surplus within the final block (424 of a declared 512 — 88 bytes, less than one block) and so cannot see this.
2	Low	core/fw_update.c:292-307, denominator at :299	eos_fw_update_progress() reports 100% while the TLV area is still outstanding. total is sizeof(eos_image_header_t) + payload_total and omits tlv_total, but total_received now counts TLV bytes too, so the ratio saturates the moment the payload completes and is then clamped at :304. Verified: after writing header + payload of the 424-byte container, state = EOS_FW_STATE_TLV, eos_fw_update_bytes_wanted() = 12, eos_fw_update_progress() = 100. Both are public API in include/eos_fw_update.h, and they now disagree about whether the transfer is finished. Pre-existing arithmetic, but this PR is what put a fourth region into the container and made the two answers diverge.	total = sizeof(eos_image_header_t) + payload_total + tlv_total, inside the same __builtin_add_overflow guard, and assert in test_fw_update.c that progress is below 100 while bytes_wanted() != 0.
3	Low	core/fw_transport_uart.c:238-242 (XMODEM), :356-366 (YMODEM)	Both receivers return EOS_OK on EOT no matter how much of the container is still outstanding. Verified: three of the container's four blocks then EOT gives receive_rc = 0 with state = EOS_FW_STATE_PAYLOAD and bytes_wanted = 40, on both transports. End-to-end this is fail-closed — eos_fw_transport_update() (:41) calls finalize(), which returns EOS_ERR_INVALID because the state is not VERIFY — so it is Low, not a security defect. It is filed because this commit hardened the opposite direction on both transports, eos_fw_update_bytes_wanted() makes the check one line, and both eos_fw_transport_uart_xmodem() and ..._ymodem() are exported from include/eos_fw_transport.h, so a caller that drives ops->receive itself is told a truncated transfer succeeded.	At each EOT, before breaking: if (eos_fw_update_bytes_wanted(ctx) != 0) return EOS_ERR_INVALID;. Send the ACK first so the sender still sees a completed handshake.
4	Low	PR body — "Testing Environment" and "Transport handling"	The body has gone stale against its own head. It claims eboot_test_fw_transport — 13/13 passed; this head has 15 tests and reports 15/15 (I ran it). The Transport handling section describes only the previous head's work — the XMODEM container clamp and the YMODEM block-0 size hardening — and does not mention this commit's two behavioural changes: the new hard rejection of a data block after the container (core/fw_transport_uart.c:275-284) and the YMODEM switch from a file_size clamp to a container clamp (:487-497). The first is a change to what a sender may send and belongs in Compatibility, not only in Changes. Per §28 an unsupported or superseded claim is itself the finding, even where the underlying work is sound — and here it is sound; only the description is behind.	Regenerate the five test lines from a run of this head (13/13 → 15/15; the other four are still exact). Add two Transport handling bullets for the changes above, and one Compatibility line stating that an XMODEM sender which appended whole blocks past the image now gets CAN where it previously got ACK. While editing, add the Scope line the last review asked for (finding 5 below).
5	Medium (carried, untouched)	tools/imgpack.py:74, tools/eos_sign.py:105	Previous finding 3, restated because it is still open and this PR is still the one that makes the counter load-bearing. No in-tree tool emits an EOS_TLV_MIN_SEC_VER entry, so every artifact this repository can build has tlv_len == 0 and the anti-rollback floor can never be raised by an in-tree image. Coherent and fail-closed — a 0 counter can only fail against a floor, never advance one — and not a defect introduced here. It is why the feature cannot reach §28 Validated: the consumer is complete and has no producer.	Unchanged: out of scope for this PR, but say so in the body's Scope section next to the existing notes on master and the duplicated boot logic. The work itself is a --min-sec-ver N option on both packers plus a round-trip test through eos_rollback_read_image_counter(), and it is its own PR.
6	Low (carried, untouched)	core/secure_boot.c:134, stage1/jump_app.c:66, core/fw_update.c:259	Previous finding 7. Three copies of read → verify → stage, and eos_secure_boot() still has no caller anywhere in stage1/. Restated only so it is not dropped. Finding 1 is once again a divergence between two receivers of the same container — the third head in a row where one block transport was taught something the other was not.	Unchanged: extract into one function in core/rollback.c and have all three call it. Separate PR — .ai/architect.md forbids folding a structural change into a behavioural one.
7	Low (carried, untouched)	test_fw_transport.c:768, test_fw_update.c:307, test_tlv_auth.c:364, test_jump_app.c:268	Previous finding 8. Four hand-maintained tests_run literals; all four assign after the run, so a forgotten bump reports a spurious failure rather than a silent pass — the safe direction. The transport literal was correctly bumped 13 → 15 this commit, which is also what makes the PR body's stale 13/13 visible.	No change here. eBoot#95 replaces this org-wide and is still open; rebase after it lands.

Architecture conformance

Conforms, on the same reading as all four previous reviews. The new commit moves no
boundary.

• §8.1 "Rollback protection where hardware/policy supports it" and .ai/security.md
  ("anti-rollback counters actually checked"): unchanged by this commit and still correct.
  The authenticated EOS_TLV_MIN_SEC_VER is what both the boot path and the install path
  read, bound to the signed header through tlv_len/tlv_hash. Finding 5 remains the
  honest qualifier: the check is right and nothing in the tree can yet feed it a counter.
• .ai/security.md "Fail closed": honoured, and the new XMODEM rejection strengthens
  it — a sender and a receiver that disagree about the artifact now fail rather than
  proceed. Finding 1 is the gap in that, and it fails open on detection rather than open
  on content: the surplus never reaches flash and never enters the SHA/CRC, so what
  finalize() installs is still exactly the container it verified. Finding 3 is fail-closed
  end-to-end through eos_fw_transport_update() and only misreports at the ops->receive
  boundary. No security step reports success without having run.
• §5.1: no layering violation. write_container_bytes() is a static helper in the
  transport that calls down into the update core through a read-only accessor — the correct
  direction. Nothing points up a tier. TCB growth this commit is ~35 lines net in
  core/fw_transport_uart.c, all of it framing logic that was previously inline in
  xmodem_receive(); the extraction removes the duplication the last review would otherwise
  have gained.
• §21: Tier 1 (Foundation). Install-time rollback policy and the update transports
  belong in eBoot; correct repo. §21.1 is not engaged — nothing is being split out.
• §28: the four unchanged test claims in the body reproduce exactly; the fifth is stale
  (finding 4). The Scope section still states plainly what the PR does not address, which is
  the right posture, but it has not been extended for finding 5.

The design-level question behind finding 1 is not one this PR could have settled from the
document: §8.1 as amended by the proposal filed against b8363040 says a length-less
transport must "stop offering bytes when the answer is zero while still completing its own
handshake", which describes YMODEM's behaviour here and would forbid XMODEM's new CAN. The
security posture the last review asked for and this commit implemented is the opposite. An
addendum is appended for this run.

Proposed changes

1. Add the YMODEM guard (finding 1) — five lines, mirroring :275-284, plus the test case.
   It is the only item that leaves the two block transports disagreeing about the same
   protocol question for a third consecutive head.
2. Fix the progress denominator (finding 2) and the EOT completeness check (finding 3) in
   the same PR; both are one line and both are consequences of the container gaining a
   region.
3. Correct the PR body (finding 4) and add the Scope line for finding 5. No code.
4. Leave findings 5, 6 and 7 to a packer PR, a separate extraction PR, and eBoot#95.
5. Merge order, unchanged and still the last thing standing: eBoot#94 or #105 first —
   this head still does not build on its own (Not checked). Still overlaps eBoot#96 on the
   slot bound in eos_rollback_read_image_counter() and still conflicts textually with
   eBoot#104 around finalize().

I have opened no fix PR. Nothing at this head is rated High, and the policy permits fix PRs
only for High findings that are small and provable. Finding 1 is small and provable but is
Medium, and fix-verify.sh has nothing green to run against while master does not build.
I checked all 20 open PRs: none touches core/fw_transport_uart.c (confirmed by
gh pr diff --name-only for #94, #95, #96, #104, #105, #106, #107) and none adds TLV
emission to either packer, so nothing here duplicates existing work.

Not checked

• CI: no checks ran, for the fifth head running. checks.txt is empty,
  gh pr checks 103 reports no checks reported on the 'fix/eboot-test-issue' branch, and
  statusCheckRollup is []. mergeStateStatus is BLOCKED with mergeable: MERGEABLE
  and reviewDecision: REVIEW_REQUIRED, so the block is review, not conflict. Five heads of
  TCB code with zero automated evidence behind them. eBoot#90 is what would change this and
  is still open.

• This head does not compile as-is, and this PR does not cause it. Verified afresh: I
  extracted 180680cc with git archive into a scratch directory — the repository, its
  working tree (clean), its branch (fix/ed25519-low-order-keys) and its stashes were not
  touched — and ran the CI's own host configuration unmodified. It fails on
  include/eos_image.h:135,142, which assert on a reserved member that
  tlv_len/tlv_hash replaced. core/ed25519_verify.c still defines point_is_identity
  twice (:281, :338). Both are on origin/master (22d8f8b); eBoot#94 and eBoot#105 both
  fix them and both are open.

• What I did run, in a scratch copy with only those two master defects patched out and
  nothing else changed:

  $ cmake -B build/host -DEBLDR_BUILD_TESTS=ON && cmake --build build/host --parallel 4 -- -k
  $ ctest --test-dir build/host --output-on-failure --no-tests=error -E ed25519
  100% tests passed, 0 tests failed out of 22
  
  $ ./build/host/tests/eboot_test_fw_update      6/6 passed
  $ ./build/host/tests/eboot_test_tlv_auth       9/9 passed
  $ ./build/host/tests/eboot_test_jump_app       2/2 passed
  $ ./build/host/tests/eboot_test_fw_transport   15/15 passed
  $ ./build/host/tests/eboot_test_rollback       14/14 passed
  

  Four of the body's five numbers reproduce exactly; the transport suite is 15/15 at this
  head, not the 13/13 the body still claims (finding 4). That is 22 of the 23 registered
  tests. test_ed25519 is excluded because it cannot compile —
  tests/unit/test_ed25519.c:31,263,299,300 carries its own duplicate-definition merge
  artifact on master, independent of the two above.

• The ARM cross build PASSES. Same patched scratch copy, the exact configuration from
  .github/workflows/build.yml:44-52:

  $ cmake -B build-arm -DCMAKE_SYSTEM_NAME=Generic -DCMAKE_C_COMPILER=arm-none-eabi-gcc \
      -DEBLDR_BOARD=stm32f4 \
      -DCMAKE_C_FLAGS="-mcpu=cortex-m4 -mthumb -mfloat-abi=hard -mfpu=fpv4-sp-d16 -specs=nosys.specs"
  $ cmake --build build-arm --parallel 4          # exit 0
  

  ebldr_stage0.elf and eboot_firmware.elf both link. No warning or error in any file
  this PR touches, on either the host or the ARM build.

• git diff --check 22d8f8b 180680cc — PASS, exit 0. The body claims it and it holds.

• Findings 1, 2 and 3 were each verified with throwaway probes compiled into a copy of
  tests/unit/test_fw_transport.c in that scratch tree, which was deleted afterwards. None
  is offered as a test here; the findings say where the real ones belong. One of my probes
  initially reported a garbage state because I evaluated the transfer and the state read in
  the same printf argument list; the numbers quoted above are from the corrected,
  statement-sequenced version.

• XMODEM has no retransmission tolerance, and I could not settle whether it matters.
  core/fw_transport_uart.c:258 NAKs any block whose number is not expected_blk,
  including a resend of the block just written, where ymodem_receive():457 ACKs it
  without rewriting. A real sender answering that NAK would resend the same block and see
  it NAKed again. The scripted harness replays a fixed byte stream and cannot model a
  sender reacting to a NAK, so I confirmed only that the duplicate is not written twice
  (flash byte-identical, state VERIFY). Pre-existing, untouched by this PR, and not
  filed as a finding because I could not demonstrate the failure.

• Not verified on hardware. eos_hal_monotonic_read/increment on a real OTP-backed
  board is still untested — everything above is simulated flash and a simulated counter.
  §28.1 would require a named board, and finding 5 says there is not yet an artifact to put
  on one.

• The address-space question is still unsettled and I still cannot settle it. Carried
  forward from all four previous reviews: whether eos_bootctl_confirm() (core/bootctl.c,
  which calls eos_rollback_commit()) executes in the same address space as the
  eboot_jump_to_app() that staged the counter. g_staged_counter/g_staged_valid
  (core/rollback.c:17-18) are file-static RAM. If confirm runs in the application image
  the staged value is invisible there and the floor never advances. Pre-existing design;
  together with finding 5 it means neither end of the anti-rollback mechanism has been shown
  to work outside a unit test.

• slot_remain_from() still skips a slot based at address 0 (core/rollback.c:79).
  Unchanged, still latent, and untouched by this commit.

• The two .c files under poc/ were not reviewed. poc/tlv_downgrade_poc.c calls
  eos_rollback_read_image_counter() and is untouched by this diff; I did not check whether
  it still builds or still demonstrates what it claims.

• Merge order against eBoot#96 and eBoot#104 was not worked out. Both will conflict
  textually with this head — #96 on the slot bound, #104 on finalize(). I confirmed the
  overlap and did not resolve it.

• existing-comments.txt is empty, so nothing here repeats a human reviewer; there are
  still none on this PR.

---

Automated architecture review of 180680cc86c5 — 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#103 "Fix anti-rollback to use authenticated TLV counter"

head: fb978c8 author: Tanisha1723 ci: none reported

Verdict: Follow-up review (previous head 180680cc). The one new commit closes every
open code finding from the last review — the YMODEM block-after-container guard, the
progress denominator, and the EOT completeness check on both transports — and it is the
first head of this PR where the two block transports agree about the same protocol question
instead of one being taught and the other not. The PR body has been regenerated and all five
of its test claims now reproduce exactly. I ran the host suite (22/22), the ARM cross build
and git diff --check; all pass. What the commit does not reach is the third transport:
uart_raw_receive() still returns EOS_OK for a container it delivered only part of, which
is the same shape as the finding just fixed on the other two.

Previous findings — status

• Finding 1 (YMODEM accepted and discarded whole data blocks once the container was
  complete) — resolved in fb978c8e. ymodem_receive() now rejects a correctly sequenced
  block that begins with eos_fw_update_bytes_wanted(ctx) == 0
  (core/fw_transport_uart.c:481-490): CAN, EOS_ERR_INVALID. The recommended fix was taken
  verbatim, including placing the guard after the retransmit-tolerance branch at :466-473,
  so a resent final block is still ACKed rather than CANed — the ordering XMODEM does not
  need and YMODEM does. test_ymodem_data_block_after_container_is_rejected
  (tests/unit/test_fw_transport.c:616) is the case the last review asked for, and it is
  built correctly: a declared size of 640 over the 424-byte container leaves room for the
  extra block inside the file-size bound, so it exercises the container check and not the
  file_len == 0 bound. Ran it; passes.
• Finding 2 (eos_fw_update_progress() reported 100% with the TLV area outstanding) —
  resolved in fb978c8e. core/fw_update.c:302-303 folds tlv_total into the denominator
  inside the same __builtin_add_overflow guard, which is the recommended fix exactly. The
  short-circuit is correct in both directions: a wrap in either add lands on 0xFFFFFFFF.
  test_write_does_not_reach_verify_until_tlv_arrives gained the four assertions that were
  asked for, including progress() < 100 while bytes_wanted() != 0 and == 100 once the
  TLV area lands.
• Finding 3 (both receivers returned EOS_OK on EOT however much was outstanding) —
  resolved in fb978c8e, on both transports. core/fw_transport_uart.c:245 and :375
  each ACK first and then return EOS_ERR_INVALID if the container is short — the ACK-first
  ordering the last review recommended, so the sender still sees a completed handshake.
  Verified independently that the YMODEM branch also closes the EOT-before-block-0 case its
  comment claims: eos_fw_update_begin() leaves the context in EOS_FW_STATE_HEADER
  (core/fw_update.c:41), so bytes_wanted() is 156 and not 0, and the
  EOS_FW_STATE_IDLE → default → 0 arm of bytes_wanted() (:331-332) is not reached there.
  Both new tests assert !tx_contains(XM_CAN), which is the right distinction — truncation
  is an error, not a protocol abort.
• Finding 4 (PR body stale against its own head) — resolved. test_fw_transport now
  reads 18/18 and reproduces; the Changes section gained the YMODEM extra-block rejection,
  the EOT rejections and the progress denominator; Compatibility gained the two lines about
  what a sender may now no longer send. Nothing in the body overstates this head.
• Finding 5 (nothing in the repo can emit the counter this PR makes authoritative) —
  the requested treatment is done; the gap itself is still open. The Scope section now names
  tools/imgpack.py and tools/eos_sign.py explicitly and says the emission side is
  follow-up work, which is exactly what was asked for. Re-checked the code at this head:
  both packers still write b'\x00' * 30 for tlv_len(2) + tlv_hash(28)
  (tools/imgpack.py:74, tools/eos_sign.py:105), and git grep -l EOS_TLV_INFO_MAGIC
  outside tests/ and poc/ still returns only readers. Restated with new information as
  finding 3 below — eBoot#88 changes the outlook.
• Finding 6 (read → verify → stage duplicated three ways) — untouched, correctly. Still
  core/secure_boot.c:134, stage1/jump_app.c:66, core/fw_update.c:259. Restated as
  finding 4 only so it is not dropped; it is now also listed in the body's own Follow-up
  section, which is the right place for it.
• Finding 7 (tests_run hand-maintained literals) — untouched, and the transport literal
  was correctly bumped 15 → 18 (tests/unit/test_fw_transport.c:854). eBoot#95 is still open.
  No change wanted here.

Findings

#	Severity	File:line	Finding	Recommended fix
1	Low	core/fw_transport_uart.c:123-127	uart_raw_receive() is the one transport that still reports a truncated container as a successful transfer — the same defect this commit just fixed on XMODEM and YMODEM, surviving in the third receiver. It streams exactly the total_len its 4-byte prefix declared, sends UART_RAW_ACK and returns EOS_OK, with no eos_fw_update_bytes_wanted() check anywhere in the function. Verified by probe against this head: a prefix of CONT_LEN - 40 over the 424-byte TLV-carrying container gives receive_rc = 0, state = EOS_FW_STATE_PAYLOAD, bytes_wanted = 40, ACK sent. The identical shortfall over XMODEM now gives rc = -2. End-to-end it is fail-closed — eos_fw_transport_update() (:41) calls finalize(), which returns EOS_ERR_INVALID because the state is not VERIFY (I measured finalize = -2) — so this is loss of detection at the ops->receive boundary, not a bad install: hence Low, the same rating the last review gave the XMODEM and YMODEM halves before they were fixed. It matters because eos_fw_transport_uart_raw() is exported from include/eos_fw_transport.h:85 and the receive op is documented as blocking "until complete" (:46-48) without saying what complete means — a definition that now differs between the three implementations behind that one pointer. The sender's own length prefix is not an independent bound on its own bytes, which is the reasoning the commit already applies to YMODEM's file_size.	One line, mirroring :245 and :375, before the ACK at :123: if (eos_fw_update_bytes_wanted(ctx) != 0) { uint8_t nak = UART_RAW_NAK; eos_hal_uart_send(&nak, 1); return EOS_ERR_INVALID; } — NAK rather than ACK, since raw has no separate abort byte and the prefix contract is already NAK-on-reject at :92. Add the case to tests/unit/test_fw_transport.c next to test_raw_valid_transfer_is_written, which today only covers a prefix equal to the image. While editing, tighten the receive doc comment in include/eos_fw_transport.h:46-48: "complete" should say the update container is complete, so the three implementations are held to one contract rather than three.
2	Low	core/fw_transport_uart.c:527-529	Dead code in a function this commit rewrote: if (file_size > 0 && total_received >= file_size) { /* File complete — wait for EOT */ } has an empty body and no effect. It produces no compiler warning (the braces suppress -Wempty-body), so it will not be found by the build. Pre-existing and untouched by this diff — filed because the diff is what makes ymodem_receive()'s end-of-transfer handling load-bearing, and because a reader now has to work out that the real end-of-transfer decision moved to the EOT branch at :361-377.	Delete the three lines. If the comment is worth keeping, move it to the EOT branch, which is where the file-complete decision actually happens.
3	Medium (carried, treatment discharged)	tools/imgpack.py:74, tools/eos_sign.py:105	Previous finding 5. No in-tree tool emits an EOS_TLV_MIN_SEC_VER entry, so every artifact this repository can build has tlv_len == 0 and the anti-rollback floor can never be raised by an in-tree image. Coherent and fail-closed — a 0 counter can only fail against a floor, never advance one — and not a defect introduced here. It is why the feature cannot reach §28 Validated: the consumer is complete and has no producer. The documentation half of the last review's recommendation is now done, so nothing further is owed by this PR. New information, which narrows the remaining work: eBoot#88 ("fix(eos_sign): put the payload where the bootloader looks for it") is open and gives tools/eos_sign.py a real authenticated TLV area — a build_tlv() helper, tlv_len/tlv_hash parameters written into the signed prefix, and output ordered [header][payload][tlv], which is where core/rollback.c looks. Its TLV area carries a payload hash and a key hash, not EOS_TLV_MIN_SEC_VER, so it does not close this — but it builds the mechanism, and a follow-up written before it lands would duplicate that work.	Unchanged for this PR. For the follow-up: land eBoot#88 first, then add --min-sec-ver N as one further entry through its build_tlv() rather than a second TLV writer, plus a round-trip test through eos_rollback_read_image_counter(). tools/imgpack.py needs the same option and has no equivalent in flight.
4	Low (carried, untouched)	core/secure_boot.c:134, stage1/jump_app.c:66, core/fw_update.c:259	Previous finding 6. Three copies of read → verify → stage, and eos_secure_boot() still has no caller anywhere in stage1/. Worth noting that finding 1 above is the same shape one layer up — three receivers of one container, two of which now enforce a rule the third does not — so this is the second place in the file where a per-implementation copy is what let the behaviours drift. Restated only so it is not dropped.	Unchanged: extract into one function in core/rollback.c and have all three call it. Separate PR — .ai/architect.md forbids folding a structural change into a behavioural one.
5	Low (carried, untouched)	test_fw_transport.c:854, test_fw_update.c:318, test_tlv_auth.c:364, test_jump_app.c:268	Previous finding 7. Four hand-maintained tests_run literals; all four assign after the run, so a forgotten bump reports a spurious failure rather than a silent pass — the safe direction. Correctly bumped 15 → 18 this commit.	No change here. eBoot#95 replaces this org-wide and is still open; rebase after it lands.

Architecture conformance

Conforms, on the same reading as all five previous reviews. The new commit moves no boundary.

• §8.1 "Rollback protection where hardware/policy supports it" and .ai/security.md
  ("anti-rollback counters actually checked"): unchanged by this commit and still correct. The
  authenticated EOS_TLV_MIN_SEC_VER is what both the boot path and the install path read,
  bound to the signed header through tlv_len/tlv_hash. Finding 3 remains the honest
  qualifier: the check is right and nothing in the tree can yet feed it a counter.
• .ai/security.md "Fail closed": strengthened in three places by this commit. A short
  transfer is now an error on both block transports rather than a success the installer has
  to catch later, and a surplus frame is a protocol abort. No security step reports success
  without having run. Finding 1 is the remaining gap and it fails open on detection only —
  the truncated container never reaches EOS_FW_STATE_VERIFY, so finalize() cannot install
  it.
• §5.1: no layering violation. The three new checks all call down into the update core
  through the read-only eos_fw_update_bytes_wanted() accessor — the correct direction.
  Nothing points up a tier. TCB growth this commit is ~30 lines net in
  core/fw_transport_uart.c and 2 in core/fw_update.c, all of it rejection logic, which is
  the kind of growth "minimal and auditable" is for.
• §21: Tier 1 (Foundation). Install-time rollback policy and the update transports belong
  in eBoot; correct repo. §21.1 is not engaged — nothing is being split out.
• §28: all five of the body's test claims reproduce exactly at this head (Not checked
  below), which is the first time that has been true of the transport suite. The Scope and
  Follow-up sections state plainly what the PR does not address, including the packer gap the
  last review asked for. Nothing in the body asserts a check that was not run.
• §23.2 / API compatibility: nothing further changes here. The enum renumbering and struct
  growth were declared at 984a0d39 and are unchanged; this commit alters no public
  signature, struct layout, manifest field or serialized format. The one behavioural break to
  an out-of-tree integrator — an EOT that used to yield EOS_OK for a short transfer now
  yields EOS_ERR_INVALID — is declared in Compatibility. That is the disclosure the brief
  asks for.

No proposal is appended this run, and that is deliberate. The design question behind
finding 1 is already settled by two proposals filed against earlier heads of this same PR:
the 2026-09-05 entry "An install is verified over the whole image container and no rule says
the transport has to have delivered all of it" already requires a receiver to "report an
error for a stream that ends before the container does", and its addendum
"'stop offering bytes, but complete the handshake' gives a transport no point at which
surplus becomes an error" already states that a sender-declared length "may be used to detect
end of transfer; it may not be used to license discarding bytes". Both cover
uart_raw_receive() exactly. The design is ahead of the code here, not behind it, so finding
1 is a code finding and nothing is owed to .docx.

Proposed changes

1. Add the raw-transport completeness check and its test (finding 1), and tighten the
   receive doc comment in the same edit. One line of production code; it is the last of the
   three transports.
2. Delete the dead if at core/fw_transport_uart.c:527-529 (finding 2) while that function
   is open.
3. Leave findings 3, 4 and 5 to a packer PR sequenced after eBoot#88, a separate extraction
   PR, and eBoot#95.
4. Merge order, unchanged and still the last thing standing: eBoot#94 or #105 first —
   this head still does not build on its own (Not checked). It still overlaps eBoot#96 on the
   slot bound in eos_rollback_read_image_counter() and still conflicts textually with
   eBoot#104 around finalize(). eBoot#88 also touches include/eos_image.h and will need
   sequencing against #94/#105.

I have opened no fix PR. Nothing at this head is rated High, and the policy permits fix PRs
only for High findings that are small and provable. Finding 1 is small and provable but is
Low, and fix-verify.sh has nothing green to run against while master does not build. I
checked all 20 open PRs: none touches core/fw_transport_uart.c (confirmed by
gh pr diff --name-only for every open number), and none adds EOS_TLV_MIN_SEC_VER emission
to either packer — eBoot#88 rewrites eos_sign.py's TLV area but emits a payload hash and a
key hash, and eBoot#107 edits imgpack.py for version-component overflow only. So nothing
here duplicates existing work.

Not checked

• CI: no checks ran, for the sixth head running. checks.txt is empty, gh pr checks 103
  reports no checks reported on the 'fix/eboot-test-issue' branch, and statusCheckRollup
  is []. mergeStateStatus is BLOCKED with mergeable: MERGEABLE and reviewDecision: REVIEW_REQUIRED, so the block is review, not conflict. Six heads of TCB code with zero
  automated evidence behind them. eBoot#90 is what would change this and is still open.

• This head does not compile as-is, and this PR does not cause it. Verified afresh: I
  extracted fb978c8e with git archive into a scratch directory — the repository, its
  working tree (clean), its branch (fix/ed25519-low-order-keys) and its stashes were not
  touched — and ran the CI's own host configuration from .github/workflows/build.yml:26
  unmodified. Configure succeeds; the build fails on
  include/eos_image.h:135,142, which assert on a reserved member that tlv_len/tlv_hash
  replaced. core/ed25519_verify.c still defines point_is_identity twice (:281, :338).
  Both are on origin/master (22d8f8b); eBoot#94 and eBoot#105 both fix them and both are
  open.

• What I did run, in a scratch copy with only those two master defects patched out and
  nothing else changed:

  $ cmake -B build -DEBLDR_BUILD_TESTS=ON -DEBLDR_BOARD=none
  $ cmake --build build --parallel 4 -- -k
  $ ctest --test-dir build --output-on-failure --no-tests=error -E ed25519
  100% tests passed, 0 tests failed out of 22
  
  $ ./build/tests/eboot_test_fw_update      6/6 passed
  $ ./build/tests/eboot_test_tlv_auth       9/9 passed
  $ ./build/tests/eboot_test_jump_app       2/2 passed
  $ ./build/tests/eboot_test_fw_transport   18/18 passed
  $ ./build/tests/eboot_test_rollback       14/14 passed
  

  All five of the body's numbers reproduce exactly. That is 22 of the 23 registered tests.
  test_ed25519 is excluded because it cannot compile — tests/unit/test_ed25519.c:31,263, 299,300 carries its own duplicate-definition merge artifact on master, independent of the
  two above. The PR body's "Build / CI Status" list of base-branch blockers names the two real
  gcc ones and omits this third; it also lists two MSVC-specific items that are not gcc
  blockers. Not filed as a finding — the section's claim is that the repo-wide build is
  blocked by pre-existing base-branch defects, and that claim is true.

• The ARM cross build PASSES. Same patched scratch copy, the exact configuration from
  .github/workflows/build.yml:41-49:

  $ cmake -B build-arm -DCMAKE_SYSTEM_NAME=Generic -DCMAKE_C_COMPILER=arm-none-eabi-gcc \
      -DEBLDR_BOARD=stm32f4 \
      -DCMAKE_C_FLAGS="-mcpu=cortex-m4 -mthumb -mfloat-abi=hard -mfpu=fpv4-sp-d16 -specs=nosys.specs"
  $ cmake --build build-arm --parallel 4          # exit 0
  

  ebldr_stage0.elf and eboot_firmware.elf both link. No warning or error in any file
  this PR touches, on either the host or the ARM build — I grepped both logs for
  fw_transport_uart, fw_update.c, rollback.c and jump_app specifically.

• git diff --check 22d8f8b fb978c8e — PASS, exit 0. The body claims it and it holds.

• Finding 1 was verified with a throwaway probe compiled into a copy of
  tests/unit/test_fw_transport.c in that scratch tree, alongside a contrast probe running
  the identical shortfall over XMODEM. The scratch file was restored from git show
  afterwards and the probe is not offered as a test here; the finding says where the real one
  belongs. The repository itself was never modified.

• cppcheck (the CI lint job) — NOT RUN. It is || true in the workflow
  (.github/workflows/build.yml:73) and so cannot fail the build; I did not run it separately
  and make no claim about what it would say.

• Not verified on hardware. eos_hal_monotonic_read/increment on a real OTP-backed board
  is still untested — everything above is simulated flash and a simulated counter. §28.1 would
  require a named board, and finding 3 says there is not yet an artifact to put on one.

• The address-space question is still unsettled and I still cannot settle it. Carried
  forward from all five previous reviews: whether eos_bootctl_confirm() (core/bootctl.c,
  which calls eos_rollback_commit()) executes in the same address space as the
  eboot_jump_to_app() that staged the counter. g_staged_counter/g_staged_valid
  (core/rollback.c:17-18) are file-static RAM. If confirm runs in the application image the
  staged value is invisible there and the floor never advances. Pre-existing design; together
  with finding 3 it means neither end of the anti-rollback mechanism has been shown to work
  outside a unit test.

• XMODEM still has no retransmission tolerance, and I still could not settle whether it
  matters. core/fw_transport_uart.c:263 NAKs any block whose number is not expected_blk,
  including a resend of the block just written, where ymodem_receive():466-473 ACKs it
  without rewriting. This commit did not change that, and it deliberately placed the new
  extra-block guard after YMODEM's retransmit branch so the two do not interact — which is
  correct. The scripted harness replays a fixed byte stream and cannot model a sender reacting
  to a NAK, so this stays unfiled for the second review running: I cannot demonstrate the
  failure.

• slot_remain_from() still skips a slot based at address 0 (core/rollback.c:79).
  Unchanged, still latent, untouched by this commit.

• The two .c files under poc/ were not reviewed. poc/tlv_downgrade_poc.c calls
  eos_rollback_read_image_counter() and is untouched by this diff; I did not check whether
  it still builds or still demonstrates what it claims.

• Merge order against eBoot#96, #104 and now #88 was not worked out. All three will
  conflict or overlap — #96 on the slot bound, #104 on finalize(), #88 on
  include/eos_image.h and on the packer this PR's Scope section defers to. I confirmed the
  overlaps and did not resolve them.

• existing-comments.txt is empty, so nothing here repeats a human reviewer; there are
  still none on this PR.

---

Automated architecture review of fb978c8e1b20 — 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#103 "Fix anti-rollback to use authenticated TLV counter"

head: d722e9a author: Tanisha1723 ci: none reported

Verdict: Follow-up review (previous head fb978c8e). The one new commit
(d722e9a fix(transport): reject incomplete raw transfers) closes both open code findings
from the last review — the raw transport's completeness check and the dead if in
ymodem_receive() — and it does so in the places they were asked for, including the doc-comment
tightening that was recommended in the same edit. No new code finding. I proved the new test
is a real regression test (it fails with the guard removed, passes with it restored) and probed
the opposite direction and the one-byte boundary, both of which behave correctly. This is the
first head of this PR where all three transports behind one receive pointer enforce the same
container contract, and the header now states that contract. What the commit did not do is update
the PR body, which had been brought current at the last head and has gone stale again — including
on a behavioural break to an exported transport that Compatibility does not mention.

Previous findings — status

• Finding 1 (uart_raw_receive() reported a truncated container as a successful transfer) —
  resolved in d722e9a0. core/fw_transport_uart.c:123-130 now rejects before the ACK:
  eos_fw_update_bytes_wanted(ctx) != 0 sends UART_RAW_NAK and returns EOS_ERR_INVALID. The
  recommended fix was taken verbatim, including NAK rather than a new abort byte, matching the
  prefix contract's existing NAK-on-reject at :92. test_raw_transfer_shorter_than_container_is_rejected
  (tests/unit/test_fw_transport.c:816-829) is the case that was asked for and it is placed next to
  test_raw_valid_transfer_is_written as recommended. Verified beyond running it: I removed the
  guard alone from a scratch copy and rebuilt — the new test is the only one that fails
  (test_fw_transport.c:825: run_raw_ctx(&ctx) == EOS_ERR_INVALID), 18 of 19 still pass; restoring
  the guard returns 19/19. So the test genuinely guards the fix rather than passing incidentally.
  I also probed the boundary and the opposite direction against this head: a prefix short by a
  single byte gives rc = -2, NAK, bytes_wanted = 1; a prefix 64 bytes longer than the container
  gives rc = -2, NAK, no ACK (caught by eos_fw_update_write()'s trailing-byte rule, not the new
  guard, which is correct — bytes_wanted is 0 there); and the exact-length golden path is
  unchanged at rc = 0, finalize = 0. The raw transport is now bounded in both directions.
• Finding 2 (dead if (file_size > 0 && total_received >= file_size) { } at :527-529) —
  resolved in d722e9a0. Deleted, which is the recommended disposition. Checked the fallout the
  deletion could have had: file_size and total_received are both still live in
  ymodem_receive() (:504-505, :530), so neither becomes an unused variable, and neither the
  host nor the ARM build emits any diagnostic in this file.
• Finding 3 (nothing in the repo can emit the counter this PR makes authoritative) — untouched,
  and nothing further is owed by this PR. Re-checked at this head: tools/imgpack.py:74 and
  tools/eos_sign.py:105 still write b'\x00' * 30 for tlv_len(2) + tlv_hash(28), no .py in
  the tree mentions MIN_SEC_VER, and EOS_TLV_INFO_MAGIC outside tests/ and poc/ still
  resolves only to the reader (core/image_tlv.c, include/eos_image_tlv.h). The documentation
  half was discharged at fb978c8e and the Scope section still carries it. Restated as finding 3
  below only so it is not dropped.
• Finding 4 (read → verify → stage duplicated three ways) — untouched, correctly. Still
  core/secure_boot.c:134, stage1/jump_app.c:66, core/fw_update.c:259, and eos_secure_boot()
  still has no caller anywhere in stage1/ — stage1/jump_app.c:64 only names it in a comment.
  Restated as finding 4.
• Finding 5 (tests_run hand-maintained literals) — untouched, and the transport literal was
  correctly bumped 18 → 19 (tests/unit/test_fw_transport.c:882). eBoot#95 is still open. No change
  wanted here.

Findings

#	Severity	File:line	Finding	Recommended fix
1	Low	PR body — "Testing", "Changes", "Compatibility"	The body has gone stale against its own head again, and this time it is silent about a behavioural break. It claims test_fw_transport — 18/18 passed; this head has 19 tests and reports 19/19 (I ran it). The Changes section is titled "XMODEM/YMODEM transport handling" and describes only the previous head's work — it never mentions this commit's three changes: the raw transport's new rejection (core/fw_transport_uart.c:123-130), the receive contract now written into the public header (include/eos_fw_transport.h:45-56, the rule itself at :50-56), and the dead-code removal at the old :527-529. Compatibility is the part that matters: a raw sender whose 4-byte length prefix under-declares the container previously received UART_RAW_ACK and EOS_OK, and now receives UART_RAW_NAK and EOS_ERR_INVALID — verified by probe in both directions. eos_fw_transport_uart_raw() is exported from include/eos_fw_transport.h:93 and has no in-tree caller, so the only party this can break is an out-of-tree integrator, which is exactly the party Compatibility exists for. The body already declares the equivalent XMODEM/YMODEM break in that section, so the omission is an oversight, not a position. Per brief §5 and §8, silence about a behavioural change to a documented contract is itself the finding — the underlying work here is correct and this is only the description lagging it.	Regenerate the five test lines from a run of this head (18/18 → 19/19; the other four are still exact — I reproduced all of them). Rename the Changes subsection to cover all three transports and add three bullets: the raw completeness rejection, the tightened receive contract in the public header, and the removed dead branch. Add one Compatibility line: a raw-transport sender whose length prefix is shorter than the update container now receives NAK and EOS_ERR_INVALID where it previously received ACK and EOS_OK.
2	Low	docs/book/book.md:732-753 (ch. 7.1, 7.2)	The commit makes completeness a requirement on every implementation of ops->receive, including the out-of-tree ones ch. 7 exists to enable — §7.2's table lists Custom ("User-defined … Network, CAN bus, SPI, etc.") as a supported transport class and §7.3 walks through writing one. Nothing in ch. 7 states the rule the header now states at include/eos_fw_transport.h:50-56, so a reader implementing a custom transport from this chapter writes one that returns EOS_OK for a short stream and violates the contract with nothing to catch it. Filed narrowly: it is the contract that is newly absent here, not the listing. The eos_fw_transport_t listing at :736-743 is independently wrong on master — :740 shows int (*receive)(uint8_t *buf, size_t max_len, size_t *actual_len), which is not the real signature — and correcting that is pre-existing work this PR did not create and should not absorb. §16.1 already got the container sentence at an earlier head (:1324), so this is the one place the series has not reached.	Two sentences after the §7.1 listing, mirroring the header: a receive implementation must treat its own framing as framing only and the update container as the authority on completeness; returning EOS_OK while eos_fw_update_bytes_wanted() is non-zero is a contract violation, not a partial success. Leave the stale struct listing alone — it belongs with whichever PR repairs ch. 7.
3	Medium (carried, treatment discharged)	tools/imgpack.py:74, tools/eos_sign.py:105	Previous finding 3. No in-tree tool emits an EOS_TLV_MIN_SEC_VER entry, so every artifact this repository can build has tlv_len == 0 and the anti-rollback floor can never be raised by an in-tree image. Coherent and fail-closed — a 0 counter can only fail against a floor, never advance one — and not a defect introduced here. It is why the feature cannot reach §28 Validated: the consumer is complete and has no producer. The documentation half of the recommendation was discharged at fb978c8e and nothing further is owed by this PR. Re-confirmed against the open PR list at this run: eBoot#88 gives tools/eos_sign.py a real authenticated TLV area and mentions EOS_TLV_MIN_SEC_VER once, in a docstring explaining why tlv_len/tlv_hash being inside EOS_IMG_SIGNED_LEN is what would make such a value trustworthy — it builds the mechanism and does not emit the counter. eBoot#107 edits imgpack.py for version-component overflow only (0 hits for MIN_SEC_VER).	Unchanged for this PR. For the follow-up: land eBoot#88 first, then add --min-sec-ver N as one further entry through its TLV builder rather than a second TLV writer, plus a round-trip test through eos_rollback_read_image_counter(). tools/imgpack.py needs the same option and has nothing in flight that provides it.
4	Low (carried, untouched)	core/secure_boot.c:134, stage1/jump_app.c:66, core/fw_update.c:259	Previous finding 4. Three copies of read → verify → stage, and eos_secure_boot() still has no caller anywhere in stage1/. Worth recording that the per-implementation drift this finding is about has now been fully worked out one layer up: three receivers of one container behaved three ways across five heads and agree at this one. The same argument applies here and has not been acted on. Restated only so it is not dropped.	Unchanged: extract into one function in core/rollback.c and have all three call it. Separate PR — .ai/architect.md forbids folding a structural change into a behavioural one.
5	Low (carried, untouched)	test_fw_transport.c:882, test_fw_update.c:318, test_tlv_auth.c:364, test_jump_app.c:268	Previous finding 5. Four hand-maintained tests_run literals; all four assign after the run, so a forgotten bump reports a spurious failure rather than a silent pass — the safe direction. Correctly bumped 18 → 19 this commit, which is also what makes the body's stale 18/18 visible.	No change here. eBoot#95 replaces this org-wide and is still open; rebase after it lands.

Architecture conformance

Conforms, on the same reading as all six previous reviews. The new commit moves no boundary.

• §8.1 "Rollback protection where hardware/policy supports it" and .ai/security.md
  ("anti-rollback counters actually checked"): unchanged by this commit and still correct. The
  authenticated EOS_TLV_MIN_SEC_VER is what both the boot path and the install path read, bound to
  the signed header through tlv_len/tlv_hash. Finding 3 remains the honest qualifier: the check
  is right and nothing in the tree can yet feed it a counter.
• .ai/security.md "Fail closed": completed for the transport layer by this commit. I traced
  every EOS_OK exit from all three receivers: uart_raw_receive() returns EOS_OK only past the
  new check at :126; xmodem_receive() only via the EOT branch at :254; ymodem_receive() only
  via the EOT branch at :384. Each is gated on eos_fw_update_bytes_wanted(ctx) == 0, so the
  contract the header now asserts — "EOS_OK therefore means the whole [header][payload][TLV area]
  container was written" — holds for every implementation behind that pointer. No security step
  reports success without having run.
• §5.1: no layering violation. The new check calls down into the update core through the
  read-only eos_fw_update_bytes_wanted() accessor — the correct direction. Nothing points up a
  tier. TCB change this commit is +9 / −4 lines in core/fw_transport_uart.c: rejection logic in,
  dead code out. That is the direction "minimal and auditable" wants.
• §21: Tier 1 (Foundation). Install-time rollback policy and the update transports belong in
  eBoot; correct repo. §21.1 is not engaged — nothing is being split out.
• §23.2 / API compatibility: no public signature, struct layout, manifest field or serialized
  format changes here. What does change is the documented contract of ops->receive and the
  observable behaviour of an exported transport. The header change is the right place to declare it
  and the commit made it; the body does not carry it, which is finding 1.
• §28: four of the body's five test claims reproduce exactly at this head; the transport line is
  stale (finding 1). The Scope and Follow-up sections still state plainly what the PR does not
  address, including the packer gap, which is the right posture and unchanged.

No proposal is appended this run, and that is deliberate. The design question behind the fixed
finding was already settled by two proposals filed against earlier heads of this same PR: the
2026-09-05 entry "An install is verified over the whole image container and no rule says the
transport has to have delivered all of it" (proposals/2026-09.md:4042) requires a receiver to
"report an error for a stream that ends before the container does", and its addendum
"'stop offering bytes, but complete the handshake' gives a transport no point at which surplus
becomes an error" (:4427) states that a sender-declared length "may be used to detect end of
transfer; it may not be used to license discarding bytes" — which is the raw transport's 4-byte
prefix exactly. This commit implements both. The packer gap behind finding 3 is covered by the
2026-09-05 entry "§8.1 requires the rollback counter to be checked and never says anyone has to
write it" (:4321). The design is ahead of the code on every open question here, so nothing is owed
to the .docx.

Proposed changes

1. Correct the PR body (finding 1) — the test count, the three missing Changes bullets, and above
   all the one Compatibility line for the raw-transport break. No code.
2. Add the two-sentence contract note to book ch. 7.1 (finding 2). No code.
3. Leave findings 3, 4 and 5 to a packer PR sequenced after eBoot#88, a separate extraction PR, and
   eBoot#95.
4. Merge order, unchanged and still the last thing standing: eBoot#94 or #105 first — this head
   still does not build on its own (Not checked). It still overlaps eBoot#96 on the slot bound in
   eos_rollback_read_image_counter() and still conflicts textually with eBoot#104 around
   finalize(). eBoot#88 also touches include/eos_image.h and will need sequencing against
   #94/#105.

I have opened no fix PR. Nothing at this head is rated High, and the policy permits fix PRs only for
High findings that are small and provable; both findings here are Low and both are documentation.
fix-verify.sh also has nothing green to run against while master does not build. I checked all
19 open PRs: none touches core/fw_transport_uart.c (confirmed by gh pr diff --name-only for
every open number), and none adds EOS_TLV_MIN_SEC_VER emission to either packer. Nothing here
duplicates existing work.

Not checked

• CI: no checks ran, for the seventh head running. checks.txt is empty, gh pr checks 103
  reports no checks reported on the 'fix/eboot-test-issue' branch, and statusCheckRollup is [].
  mergeStateStatus is BLOCKED with mergeable: MERGEABLE and reviewDecision: REVIEW_REQUIRED,
  so the block is review, not conflict. Seven heads of TCB code with zero automated evidence behind
  them. eBoot#90 is what would change this and is still open.

• This head does not compile as-is, and this PR does not cause it. Verified afresh: I extracted
  d722e9a0 with git archive into a scratch directory — the repository, its working tree (clean),
  its branch (fix/ed25519-low-order-keys) and its stashes were not touched — and ran the CI's own
  host configuration from .github/workflows/build.yml:26 unmodified. Configure succeeds; the build
  fails with exit 2 on include/eos_image.h:135,142, which assert on a reserved member that
  tlv_len/tlv_hash replaced. I confirmed the attribution directly this time rather than
  inheriting it: git show origin/master:include/eos_image.h carries both the tlv_len/tlv_hash
  fields (:53-54) and the stale reserved asserts (:135,142), and this PR does not touch that
  file at all. core/ed25519_verify.c still defines point_is_identity twice (:281, :338).
  Both are on origin/master (22d8f8b); eBoot#94 and eBoot#105 both fix them and both are open.

• What I did run, in a scratch copy with only those two master defects patched out and nothing
  else changed:

  $ cmake -B build -DEBLDR_BUILD_TESTS=ON -DEBLDR_BOARD=none
  $ cmake --build build --parallel 4 -- -k
  $ ctest --test-dir build --output-on-failure --no-tests=error -E ed25519
  100% tests passed, 0 tests failed out of 22
  
  $ ./build/tests/eboot_test_fw_update      6/6 passed
  $ ./build/tests/eboot_test_tlv_auth       9/9 passed
  $ ./build/tests/eboot_test_jump_app       2/2 passed
  $ ./build/tests/eboot_test_fw_transport   19/19 passed
  $ ./build/tests/eboot_test_rollback       14/14 passed
  

  Four of the body's five numbers reproduce exactly; the transport suite is 19/19 at this head, not
  the 18/18 the body claims (finding 1). That is 22 of the 23 registered tests. test_ed25519 is
  excluded because it cannot compile — tests/unit/test_ed25519.c:31,263,299,300 carries its own
  duplicate-definition merge artifact on master, independent of the two above.

• The ARM cross build PASSES. Same patched scratch copy, the exact configuration from
  .github/workflows/build.yml:41-49:

  $ cmake -B build-arm -DCMAKE_SYSTEM_NAME=Generic -DCMAKE_C_COMPILER=arm-none-eabi-gcc \
      -DEBLDR_BOARD=stm32f4 \
      -DCMAKE_C_FLAGS="-mcpu=cortex-m4 -mthumb -mfloat-abi=hard -mfpu=fpv4-sp-d16 -specs=nosys.specs"
  $ cmake --build build-arm --parallel 4          # exit 0
  

  ebldr_stage0.elf and eboot_firmware.elf both link. No warning or error in any file this PR
  touches, on either the host or the ARM build — I grepped both logs for fw_transport_uart,
  fw_update.c, rollback.c, jump_app and image_verify specifically.

• git diff --check 22d8f8b d722e9a0 — PASS, exit 0. The body claims it and it holds.

• The guard-removal experiment and the three raw-transport probes were done in that scratch
  tree. The production file and the test file were each backed up, modified, and restored from the
  backup, and the suite was re-run after each restore to confirm 19/19. The probes are not offered
  as tests here; the finding-1 resolution note says what they showed. The repository itself was
  never modified — its working tree was clean at the start of this run and is clean now.

• cppcheck (the CI lint job) — NOT RUN. It is || true in the workflow
  (.github/workflows/build.yml:73) and so cannot fail the build; I did not run it separately and
  make no claim about what it would say.

• Not verified on hardware. eos_hal_monotonic_read/increment on a real OTP-backed board is
  still untested — everything above is simulated flash and a simulated counter. §28.1 would require a
  named board, and finding 3 says there is not yet an artifact to put on one.

• The address-space question is still unsettled and I still cannot settle it. Carried forward
  from all six previous reviews: whether eos_bootctl_confirm() (core/bootctl.c, which calls
  eos_rollback_commit()) executes in the same address space as the eboot_jump_to_app() that
  staged the counter. g_staged_counter/g_staged_valid (core/rollback.c:17-18) are file-static
  RAM. If confirm runs in the application image the staged value is invisible there and the floor
  never advances. Pre-existing design; together with finding 3 it means neither end of the
  anti-rollback mechanism has been shown to work outside a unit test.

• XMODEM still has no retransmission tolerance, and I still could not settle whether it matters.
  core/fw_transport_uart.c:272 NAKs any block whose number is not expected_blk, including a
  resend of the block just written, where ymodem_receive():477-482 ACKs it without rewriting. This
  commit did not change either. The scripted harness replays a fixed byte stream and cannot model a
  sender reacting to a NAK, so this stays unfiled for the third review running: I cannot demonstrate
  the failure.

• slot_remain_from() still skips a slot based at address 0 (core/rollback.c:79). Unchanged,
  still latent, untouched by this commit.

• The two .c files under poc/ were not reviewed. poc/tlv_downgrade_poc.c calls
  eos_rollback_read_image_counter() and is untouched by this diff; I did not check whether it still
  builds or still demonstrates what it claims.

• Merge order against eBoot#96, #104 and #88 was not worked out. All three will conflict or
  overlap — #96 on the slot bound, #104 on finalize(), #88 on include/eos_image.h and on the
  packer this PR's Scope section defers to. I confirmed the overlaps and did not resolve them.

• existing-comments.txt is empty, so nothing here repeats a human reviewer; there are still
  none on this PR after seven heads.

---

Automated architecture review of d722e9a0ae53 — 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#103 "Fix anti-rollback to use authenticated TLV counter"

head: 0aec419 author: Tanisha1723 ci: none reported

Verdict: Follow-up review (previous head d722e9a0). Both open findings from the
last review are resolved and there is no new finding. The one new commit
(0aec419 docs: document transport completeness contract) is documentation only —
docs/book/book.md +8/−0, no code changed at all — and it closes finding 2 in the place
it was asked for. Finding 1 was closed out-of-band by an edit to the PR body, which I
re-verified line by line against a fresh run of this head rather than taking it on trust.
This is the eighth head of this PR and the first at which nothing is outstanding that the
PR itself owes. What remains is entirely external: it still does not build on origin/master
through no fault of its own, it still has zero CI, and it still needs sequencing behind
three other open PRs.

Previous findings — status

• Finding 1 (PR body stale against its own head, and silent on a behavioural break to an
  exported transport) — resolved. Every part of the recommendation was taken. The
  Testing section now reads test_fw_transport — 19/19 passed; I ran all five suites at
  this head and all five numbers reproduce exactly (6/6, 9/9, 2/2, 19/19, 14/14 —
  command output under "What I ran"). The Changes subsection was renamed from
  "XMODEM/YMODEM transport handling" to "Transport completeness — XMODEM, YMODEM and raw"
  and gained the three missing bullets: the raw completeness rejection
  (core/fw_transport_uart.c:123-130), the receive contract now stated in the public
  header (include/eos_fw_transport.h:45-56), and the removed no-op YMODEM branch. Most
  importantly Compatibility now carries the line that was missing: "A raw transfer whose
  declared length is shorter than the complete update container is now rejected with
  UART_RAW_NAK and EOS_ERR_INVALID instead of the previous UART_RAW_ACK and EOS_OK."
  That was the substance of the finding — the break to eos_fw_transport_uart_raw() is now
  declared alongside the XMODEM/YMODEM ones it belongs with.
• Finding 2 (book ch. 7 never states the completeness contract the header now imposes on
  every ops->receive, including the custom transports ch. 7 exists to enable) — resolved
  in 0aec419e. docs/book/book.md:745-752, immediately after the §7.1 listing and
  before §7.2, exactly where it was recommended. I checked the prose against the header
  rather than just its presence: the book says framing "bounds the transfer, but the update
  container is authoritative for completeness", that eos_fw_update_bytes_wanted() reports
  what is outstanding, and that returning EOS_OK with a non-zero count "is a contract
  violation rather than a partial success" — which is the header's rule at
  include/eos_fw_transport.h:50-56 in the header's own terms. I also checked the doc's
  factual claim against the implementation, not just against the other doc:
  eos_fw_update_bytes_wanted() (core/fw_update.c:313-334) does return the outstanding
  remainder of [header][payload][TLV area] in each of the three receiving states —
  HEADER returns the header remainder (payload and TLV lengths are not yet known, which
  the code comments), PAYLOAD returns payload plus TLV remainder, TLV returns the TLV
  remainder. The sentence is accurate. As recommended, the commit left the pre-existing
  stale eos_fw_transport_t listing at :736-743 alone, and the body's Follow-up section
  now names that cleanup explicitly as separate work.
• Finding 3 (nothing in the repo can emit the counter this PR makes authoritative) —
  untouched, and nothing further is owed by this PR. Re-confirmed at this head:
  tools/imgpack.py:74 and tools/eos_sign.py:105 still write b'\x00' * 30 for
  tlv_len(2) + tlv_hash(28), and MIN_SEC_VER appears in zero .py files in the tree.
  Restated as finding 3 below only so it is not dropped.
• Finding 4 (read → verify → stage duplicated three ways) — untouched, correctly.
  Still core/secure_boot.c:134, stage1/jump_app.c:66, core/fw_update.c:259, and
  eos_secure_boot() still has no caller outside tests/. Restated as finding 4.
• Finding 5 (tests_run hand-maintained literals) — untouched, correctly. All four
  still hand-assigned (test_fw_update.c:318, test_tlv_auth.c:364, test_jump_app.c:268,
  test_fw_transport.c:882, the last still 19). eBoot#95 is still open. No change wanted.

Findings

#	Severity	File:line	Finding	Recommended fix
3	Medium (carried, treatment discharged)	tools/imgpack.py:74, tools/eos_sign.py:105	Previous finding 3, unchanged. No in-tree tool emits an EOS_TLV_MIN_SEC_VER entry, so every artifact this repository can build has tlv_len == 0 and the anti-rollback floor can never be raised by an in-tree image. Fail-closed and coherent — a 0 counter can only fail against a floor, never advance one — and not a defect this PR introduced. It is why the feature cannot reach §28 Validated: the consumer is complete and has no producer. The documentation half of the recommendation was discharged at fb978c8e and the Scope section still carries it, so nothing further is owed by this PR. Re-confirmed against the open PR list this run: eBoot#88 builds a real authenticated TLV area in tools/eos_sign.py but does not emit the counter; eBoot#107 touches imgpack.py for version-component overflow only.	Unchanged for this PR. Follow-up: land eBoot#88 first, then add --min-sec-ver N as one further entry through its TLV builder rather than a second TLV writer, plus a round-trip test through eos_rollback_read_image_counter(). tools/imgpack.py needs the same option and has nothing in flight that provides it.
4	Low (carried, untouched)	core/secure_boot.c:134, stage1/jump_app.c:66, core/fw_update.c:259	Previous finding 4, unchanged. Three copies of read → verify → stage.	Unchanged: extract into one function in core/rollback.c and have all three call it. Separate PR — .ai/architect.md forbids folding a structural change into a behavioural one.
5	Low (carried, untouched)	test_fw_transport.c:882, test_fw_update.c:318, test_tlv_auth.c:364, test_jump_app.c:268	Previous finding 5, unchanged. Four hand-maintained tests_run literals; all four assign after the run, so a forgotten bump reports a spurious failure rather than a silent pass — the safe direction.	No change here. eBoot#95 replaces this org-wide and is still open; rebase after it lands.

No new finding is filed at this head. The new commit adds no code, and I re-read the
transport code it documents rather than assuming the last review's conclusion still held:
I traced every EOS_OK exit from all three receivers again and each is still gated on
eos_fw_update_bytes_wanted(ctx) == 0 (uart_raw_receive():126, xmodem_receive():254,
ymodem_receive():384). I specifically probed whether the default: return 0 arm of
eos_fw_update_bytes_wanted() — which covers ERROR as well as COMPLETE — could let a
failed write reach a completeness check that then reads as complete. It cannot: every
eos_fw_update_write() / write_container_bytes() failure returns immediately with NAK or
CAN before any completeness check (core/fw_transport_uart.c:111-116, :302-307, :522-527),
so no errored context can reach an EOS_OK exit. Fail-closed holds.

Architecture conformance

Conforms, on the same reading as all seven previous reviews. A documentation-only commit
moves no boundary; I record the sections rather than re-arguing them.

• §8.1 "Rollback protection where hardware/policy supports it" and .ai/security.md
  ("anti-rollback counters actually checked"): unchanged. The authenticated
  EOS_TLV_MIN_SEC_VER is what both the boot path and the install path read, bound to the
  signed header through tlv_len/tlv_hash. Finding 3 remains the honest qualifier.
• §8.1 "Explicit separation between implemented, experimental and planned security
  features": this is the requirement the last two heads have been serving, and this
  commit completes it. The rule now exists in the public header, in book §7.1 for the
  custom-transport author, and in book §16.1 (:1332) for the container itself — the three
  places a reader can arrive from. The PR body's Scope and Follow-up sections still state
  plainly what is not implemented, including the packer gap.
• .ai/security.md "Fail closed": unchanged and re-verified above.
• §5.1: no layering violation; nothing points up a tier. TCB delta at this commit is
  zero lines — documentation only. That is the direction "minimal and auditable" wants.
• §21: Tier 1 (Foundation). Install-time rollback policy and update transports belong
  in eBoot; correct repo. §21.1 not engaged.
• §23.2 / API compatibility: no public signature, struct layout, manifest field or
  serialized format changes at this head. The behavioural break to eos_fw_transport_uart_raw()
  landed at d722e9a0, is declared in the header, and is now declared in the PR body's
  Compatibility section as well (finding 1, resolved).
• §28: all five of the body's test claims now reproduce exactly at this head. The
  build/CI section of the body is honest about what does not build and why.

No proposal is appended this run, and that is deliberate. The design questions behind
this PR were settled by three proposals already filed against earlier heads — the
2026-09-05 entries at proposals/2026-09.md:4042 (a receiver must error on a stream that
ends before the container does), :4427 (a sender-declared length "may be used to detect
end of transfer; it may not be used to license discarding bytes"), and :4321 (§8.1
requires the counter to be checked and never says anyone has to write it). This commit
documents the first two; nothing in it reveals a new gap in the master design.

Proposed changes

1. Nothing on this PR. Findings 1 and 2 are resolved and findings 3, 4 and 5 are not
   this PR's to fix. From the author's side this branch is finished.
2. Merge order — the only thing still standing, and all of it external:
   • eBoot#94 or #105 must land first. This head does not build on its own, and does
     not cause that (evidence below). Both PRs fix the two origin/master defects.
   • eBoot#96 touches stage1/jump_app.c and tests/CMakeLists.txt; eBoot#104
     touches core/fw_update.c and tests/CMakeLists.txt. This PR touches all three
     files, so both will conflict textually. Verified by gh pr diff --name-only this run.
   • eBoot#88 touches include/eos_image.h and tools/eos_sign.py; it will need
     sequencing against #94/#105 rather than against this PR, and it is the prerequisite
     for the finding-3 follow-up.
   • No open PR touches core/fw_transport_uart.c or docs/book/book.md — I checked all
     19 other open PRs by name-only diff. Nothing here duplicates work in flight.

I have opened no fix PR. There is no High finding at this head, and the policy permits fix
PRs only for High findings that are small and provable. The two master build defects are
High in their own right but are already fixed by two open PRs (#94, #105), so opening a
third would be exactly the duplicate the brief forbids.

Not checked

• CI: no checks ran, for the eighth head running. checks.txt is empty,
  gh pr checks 103 reports no checks reported on the 'fix/eboot-test-issue' branch, and
  statusCheckRollup is []. mergeStateStatus is BLOCKED with mergeable: MERGEABLE
  and reviewDecision: REVIEW_REQUIRED, so the block is review, not conflict. Eight heads
  of TCB code with zero automated evidence behind them. eBoot#90 is what would change this
  and is still open.

• This head does not compile as-is, and this PR does not cause it. Verified afresh at
  this head, not inherited. I extracted 0aec419e with git archive into a scratch
  directory — the repository, its clean working tree, its branch
  (fix/ed25519-low-order-keys) and its stashes were never touched — and ran the CI's own
  host configuration from .github/workflows/build.yml:26-31 unmodified. Configure
  succeeds; the build fails exit 2 on include/eos_image.h:135,142, which assert on a
  reserved member that tlv_len/tlv_hash replaced. This PR does not touch that file.
  core/ed25519_verify.c still defines point_is_identity twice (:281, :338). Both
  defects are on origin/master (22d8f8b) — I confirmed by reading both files at that
  commit directly. eBoot#94 and eBoot#105 both fix them and both are open.

• What I ran, in a scratch copy with only those two master defects patched out and
  nothing else changed:

  $ cmake -B build -DEBLDR_BUILD_TESTS=ON -DEBLDR_BOARD=none      # exit 0
  $ cmake --build build --parallel 4 -- -k
  $ ctest --test-dir build --output-on-failure --no-tests=error -E ed25519
  100% tests passed, 0 tests failed out of 22
  
  $ ./build/tests/eboot_test_fw_update      6/6 passed
  $ ./build/tests/eboot_test_tlv_auth       9/9 passed
  $ ./build/tests/eboot_test_jump_app       2/2 passed
  $ ./build/tests/eboot_test_fw_transport   19/19 tests passed
  $ ./build/tests/eboot_test_rollback       14/14 tests passed
  

  All five of the body's numbers reproduce exactly — the stale 18/18 of the last head is
  gone. That is 22 of the 23 registered tests. test_ed25519 is excluded because it cannot
  compile: tests/unit/test_ed25519.c:31,263,299,300 carries its own duplicate-definition
  merge artifact on master, independent of the two defects above and untouched by this PR.

• The ARM cross build PASSES. Same patched scratch copy, the exact configuration from
  .github/workflows/build.yml:41-49:

  $ cmake -B build-arm -DCMAKE_SYSTEM_NAME=Generic -DCMAKE_C_COMPILER=arm-none-eabi-gcc \
      -DEBLDR_BOARD=stm32f4 \
      -DCMAKE_C_FLAGS="-mcpu=cortex-m4 -mthumb -mfloat-abi=hard -mfpu=fpv4-sp-d16 -specs=nosys.specs"
  $ cmake --build build-arm --parallel 4          # exit 0
  

  ebldr_stage0.elf and eboot_firmware.elf both link. I grepped both build logs for
  fw_transport_uart, fw_update.c, rollback.c, jump_app and image_verify: no
  warning or error in any file this PR touches, on either build. The ARM log does carry
  pre-existing -Wformat warnings in core/power_init.c:84, core/ecc_scrub.c:65 and
  core/bmc_handoff.c:55 — none of those files is in this diff.

• git diff --check 22d8f8b 0aec419e — PASS, exit 0. The body claims it and it holds.

• Two of the four base-branch blockers the body lists are NOT VERIFIED by me. I
  confirmed the duplicate point_is_identity and the stale reserved[] asserts directly.
  The other two are MSVC-specific: I have no MSVC toolchain here and no workflow in
  .github/workflows/build.yml exercises one, so /std:clatest in core/keystore.c and
  the __builtin_add_overflow warning handling are unchecked claims either way. I did
  check that __builtin_add_overflow is genuinely pre-existing rather than introduced
  here — origin/master:core/fw_update.c:222 already uses it, so the body's "existing" is
  the correct characterisation.

• cppcheck (the CI lint job) — NOT RUN. It is || true in the workflow
  (.github/workflows/build.yml:73) and so cannot fail the build; I did not run it
  separately and make no claim about what it would say.

• Not verified on hardware. eos_hal_monotonic_read/increment on a real OTP-backed
  board is still untested — everything above is simulated flash and a simulated counter.
  §28.1 would require a named board, and finding 3 says there is not yet an artifact to put
  on one.

• The address-space question is still unsettled and I still cannot settle it. Carried
  from all seven previous reviews: whether eos_bootctl_confirm() (core/bootctl.c, which
  calls eos_rollback_commit()) executes in the same address space as the
  eboot_jump_to_app() that staged the counter. g_staged_counter/g_staged_valid
  (core/rollback.c:17-18) are file-static RAM. If confirm runs in the application image
  the staged value is invisible there and the floor never advances. Pre-existing design;
  together with finding 3 it means neither end of the anti-rollback mechanism has been
  shown to work outside a unit test.

• XMODEM still has no retransmission tolerance, and I still could not settle whether it
  matters. core/fw_transport_uart.c:272 NAKs any block whose number is not
  expected_blk, including a resend of the block just written, where
  ymodem_receive():477-482 ACKs it without rewriting. Unchanged by this commit. The
  scripted harness replays a fixed byte stream and cannot model a sender reacting to a NAK,
  so this stays unfiled for the fourth review running: I cannot demonstrate the failure.

• slot_remain_from() still skips a slot based at address 0 (core/rollback.c:79).
  Unchanged, still latent, untouched by this commit.

• The two .c files under poc/ were not reviewed. poc/tlv_downgrade_poc.c calls
  eos_rollback_read_image_counter() and is untouched by this diff; I did not check
  whether it still builds or still demonstrates what it claims.

• Book ch. 7 is still wrong about the receive signature, and I did not re-file it.
  docs/book/book.md:740 still shows int (*receive)(uint8_t *buf, size_t max_len, size_t *actual_len) and the §7.3 and §7.4 examples are written against that shape, so the new
  contract paragraph now sits between a listing and an example that contradict it. This is
  pre-existing master breakage that this PR did not create, the last review explicitly
  ruled it out of scope, and the body's Follow-up section now names it as separate work.
  Recording it here so whoever repairs ch. 7 knows the contract paragraph is already in
  place and needs no second copy.

• Merge conflicts against eBoot#96 and #104 were confirmed but not resolved. I verified
  the file overlaps by name-only diff and did not attempt a trial merge.

• existing-comments.txt is empty, so nothing here repeats a human reviewer. There are
  still none on this PR after eight heads.

---

Automated architecture review of 0aec419ee5e5 — 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.