fix: repo does not build at all with EBLDR_BUILD_TESTS=ON

[sumit-2304]

Summary

Type of Change

• eat — New feature
• ix — Bug fix
• docs — Documentation only
• style — Formatting, no code change
• [ ]
  efactor — Code restructuring without behavior change
• est — Add or fix tests
• �uild — Build system or dependency changes
• ci — CI/CD pipeline changes
• perf — Performance improvement

Changes

Testing

• Unit tests pass (ctest --test-dir build --output-on-failure)
• Integration tests pass
• Manual testing performed
• New tests added for new functionality

Pre-Submission Checklist

• Code compiles without warnings (-Wall -Wextra -Werror for C)
• All existing tests pass
• New tests added for new functionality
• Documentation updated if API changed
• Commit messages follow (): convention
• Branch is rebased on latest master

Related Issues

Additional Notes

[srpatcha]

Review — eBoot#105 "fix: repo does not build at all with EBLDR_BUILD_TESTS=ON"

head: 71d22d9 author: sumit-2304 ci: none reported (gh pr checks 105 → "no checks reported on the branch"; statusCheckRollup is empty)

Verdict: The defect is real and this fixes it. master genuinely does not compile — I ran it — and at this head the host build and the full ctest suite are clean. The problem is that open PR #94 fixes the same three files from the same cause and was opened a day earlier, and where the two differ, #94's version of the header assert is the stronger one.

Findings

#	Severity	File:line	Finding	Recommended fix
1	Medium	core/ed25519_verify.c, include/eos_image.h, tests/unit/test_ed25519.c	Duplicate of open PR #94 ("fix: repair master — the ABI asserts and the Ed25519 verifier both merged broken", head f704d87f, opened 2026-09-03T09:37Z, this one 2026-09-04T09:22Z). Same three files, same defect, same cause. Both branch directly off 22d8f8b9 = current origin/master, so whichever lands first makes the other a conflicting no-op — and it is master being unbuildable that they both fix, so the second one to be looked at will look unnecessary rather than conflicting.	Close one. On the merits of the one place they diverge (finding 2), #94 is the one to keep; this PR's contribution is then already covered. If #105 is preferred instead, take #94's two width asserts into it first.
2	Low	include/eos_image.h:148-150	EOS_IMG_STATIC_ASSERT(sizeof(((eos_image_header_t *)0)->tlv_hash) == EOS_IMG_TLV_HASH_LEN, ...) cannot fail. The field is declared uint8_t tlv_hash[EOS_IMG_TLV_HASH_LEN] at :54, so the assert compares the macro against itself. The block it sits in exists specifically to pin widths that offsets cannot see, and this one pins nothing. The line it replaced, sizeof(reserved) == 30, was a literal and did. This is not a hole — verified: setting EOS_IMG_TLV_HASH_LEN to 26 fails the build, but on "signature[] must stay at offset 92" and "the signed prefix is the 92 bytes before signature[]", never on this assert — so the wire format stays pinned by its neighbours. It is a check that reads as coverage and provides none.	Use literals, as #94 does: sizeof(tlv_len) == 2 and sizeof(tlv_hash) == 28. Two asserts, both able to fail, together pinning the same 30 bytes the old reserved[30] assert did.
3	Low	PR body	The body is the unmodified template. No summary, no entry under "Changes", no "Type of Change" ticked, and every box under "Testing" and "Pre-Submission Checklist" left unchecked — including "Unit tests pass" and "Code compiles without warnings". Per the review brief §5 and .github/CONTRIBUTING.md, a build-repair PR has to say what was run. Nothing here is claimed falsely; nothing is claimed at all.	Fill in the body with the cmake/cmake --build/ctest invocations and their results. My own runs are in "Verification I ran" below and can be quoted.
4	Low	PR title	The title scopes the breakage to EBLDR_BUILD_TESTS=ON; it is wider than that. Verified: origin/master with the default EBLDR_BUILD_TESTS=OFF also fails, with 12 errors, all from include/eos_image.h:135 and :142 referring to a reserved member that eos_image_header_t has not had since #93. The header is public and reaches eboot_core, so the default host build is broken too. This PR fixes that as well — the title just undersells it.	Retitle to something like "fix: repair master — eos_image.h names a removed field and ed25519_verify.c has duplicate definitions". It matters because someone triaging "is master broken for me?" will read the current title as "only if you enable tests".

Nothing else. The rest of the change is correct and the reasoning in it is sound:

• core/ed25519_verify.c — master defines point_is_identity() twice (:281 and :338) and key_has_prime_order() is dead alongside the equivalent public_key_is_valid_subgroup(). This keeps one copy of each, keeps the subgroup check itself intact (:316-328, still both conditions: [L]A == identity && A != identity), and folds the surviving comment together without losing the reason either condition is there.
• tests/unit/test_ed25519.c — master defines TEST(test_ed25519_identity_key_forgery_rejected) twice (:200, :263), calls it twice from main(), never calls test_ed25519_low_order_R_with_a_valid_key_is_not_a_forgery(), and references k_low_order[] and messages[] which do not exist in the file. No assertion is lost here: the deleted definition is the second copy, and the call it replaced was the second call. Net, one test stops running twice and one that never ran starts running.
• The restored k_low_order[8][32] table is correct. All eight entries are the canonical small-order encodings of edwards25519, and the internal cross-check the comment claims holds: entries 0/6 (…fc85/…fc05), 1/5 (…0080/all-zero) and 2/4 (…ac03fa/…ac037a) differ only in bit 7 of byte 31, and entry 3 (ecff…7f, the order-2 point) is its own negation.
• tests_run = 13 matches 13 TEST() definitions and 13 run_*() calls at this head. On master the literal said 11 against 13 definitions.

Architecture conformance

Conforms.

• §5.1 architectural law — holds. Three files inside eBoot, no new include, link or manifest edge in any direction. core/ and include/ are where the verifier and the wire-format contract belong per .ai/architect.md's target layout; nothing moved.
• §8.1 required boot concepts / §14.1 — preserved rather than touched. "Signed manifests and images" depends on public_key_is_valid_subgroup() still rejecting low-order keys, and it does; the deletion in ed25519_verify.c removes the duplicate of that logic, not the logic. No primitive is invented (§14.1) — this is repair of a merge, not new crypto.
• §21 tier placement — Tier 1 Foundation, eBoot. Correct repo.
• §28 status and claims policy — this is where the PR falls short, via finding 3: it makes no claim and offers no evidence for a change to TCB code. .ai/security.md's ground rule ("the firmware building proves nothing about its security. Every claim here needs a check that was actually run") cuts both ways — the runs exist, they are just not written down.

No architecture proposal from this PR. The master design is not wrong here; a merge was.

Proposed changes

Smallest sequence that keeps things working:

1. Decide between this and #94 before anything else — everything below is moot if #94 lands.
2. If this one is kept, replace include/eos_image.h:148-150 with the two literal asserts (finding 2):

   EOS_IMG_STATIC_ASSERT(sizeof(((eos_image_header_t *)0)->tlv_len) == 2,
                         "tlv_len is 2 bytes on the wire");
   EOS_IMG_STATIC_ASSERT(sizeof(((eos_image_header_t *)0)->tlv_hash) == 28,
                         "tlv_hash is 28 bytes on the wire");

3. Fill in the PR body and widen the title (findings 3, 4).

Optional, and not this PR's to carry: TEST(test_ed25519_low_order_R_with_a_valid_key_is_not_a_forgery) sits below the /* ---- SHA-512 ... ---- */ banner at :303, so an Ed25519 test is filed under the SHA-512 section. That misplacement is inherited from master (:276/:278), not introduced here.

Verification I ran

git archive of the PR head into a scratch directory — the working checkout at /home/srpatcha/eos/eBoot was not touched and stayed clean. Host GCC, cmake 3.x.

• PR head, configure: cmake -S . -B build -DEBLDR_BUILD_TESTS=ON → rc 0.
• PR head, build: cmake --build build -j4 → rc 0. The only diagnostic in the whole build is the intentional #warning at core/keystore.c:29 about the RFC 8032 test-vector trust anchor. Nothing from any of the three changed files.
• PR head, tests: ctest --test-dir build --output-on-failure --no-tests=error → rc 0, 21/21 passed, including test_ed25519 (6.85 s).
• master, tests on: same commands on origin/master (22d8f8b9) → build rc 2. Errors: include/eos_image.h:135 and :142 'eos_image_header_t' has no member named 'reserved'.
• master, tests off (finding 4): cmake -S . -B build2 with the default EBLDR_BUILD_TESTS=OFF → build rc 2, 12 errors, same origin. Master is broken in the default configuration.
• Finding 2: edited EOS_IMG_TLV_HASH_LEN from 28 to 26 in the scratch tree and rebuilt eboot_core. Failed — on "signature[] must stay at offset 92" and "the signed prefix is the 92 bytes before signature[]". The tlv_hash width assert did not fire. Restored the file afterwards.
• Finding 1: gh pr list --state open and gh pr view 94 --json files. #94 touches core/ed25519_verify.c, include/eos_image.h, tests/unit/test_ed25519.c — exactly these three. git merge-base 71d22d9 origin/master == git rev-parse origin/master, so this head is directly on top of current master and the build result above is the merge result.
• Table correctness: compared all eight k_low_order entries byte-for-byte against the sign-bit relation the comment asserts, and against the canonical small-order encoding set.

Not checked

• CI. There is none on this branch to check. gh pr checks 105 reports no checks and statusCheckRollup is empty, so every result above is mine and none of it is reproduced by the project. Open PR #90 ("ci: add one job branch protection can require") is the existing work on that; I am not raising it against this PR.
• -Werror. The PR template asks the author to confirm "compiles without warnings (-Wall -Wextra -Werror for C)". CMakeLists.txt:37 and :42 set -Wall -Wextra and not -Werror, so that box could not be honestly ticked as written and my clean build does not prove what the template implies. Repo-level gap, not this PR's; PRs #101 and #90 are in that area.
• Cross builds. Host only. I did not configure any board or toolchain file, so I have not shown the header change is clean for a cross target. Both changed non-test files are target-independent C, so I expect no difference — that is inference, not a run. Unknown.
• Whether #94 is the better fix overall. I compared the two heads on include/eos_image.h in full and read the file lists. I did not review #94's 153-line test diff against this one's 42, so "keep #94" in finding 1 rests on the header comparison alone.
• The order of the eight low-order encodings against the derivation the comment describes. I verified the set and the sign-bit pairing. I did not re-run unpackneg()/point_add() to confirm the entries are in 1G..8G order as claimed, so the per-entry order annotations (1, 2, 4, 4, 8, 8, 8, 8) are unverified.
• git log for secrets. Not applicable to this diff and not run.

---

Automated architecture review of 71d22d96582c — 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.