chore(tests): drop the unused assert.h include from test_net.c by Kartikey1306 · Pull Request #116 · embeddedos-org/eos

Kartikey1306 · 2026-09-01T15:05:19Z

CI — eos has been red on master since 9861a3b. Two independent breakages, both fixed here.

1. The default configuration does not compile

net/src/net_posix.c:43:30: error: unknown type name 'eos_net_addr_t'
net/src/net_posix.c:73:29: error: unknown type name 'eos_net_proto_t'

net/include/eos/net.h wraps everything it declares in #if EOS_ENABLE_NET. Nothing sets that flag unless a product profile selects it or EOS_BUILD_TESTS AND NOT EOS_PRODUCT turns it on. But CMake adds the source unconditionally:

if(NOT CMAKE_CROSSCOMPILING)
    target_sources(eos_net PRIVATE net/src/net_posix.c)

EOS_ENABLE_NET never enters that decision, so on any host build without tests or a product the header expands to nothing while net_posix.c still defines the API against it.

net.c already carries #if EOS_ENABLE_NET around its body and survives untouched; the POSIX mapping was the half that never got the guard. Added, so the two translation units appear and disappear together.

Worth noting: the Default configuration job's own comment says it exists because "that gap let a translation unit that only compiles under EOS_ENABLE_NET reach master." It was added for exactly this class, and it caught exactly this.

2. `test_net` does not link

undefined reference to `assert'

test_net.c defines CHECK() and explains why in a comment above it — CI builds Release, NDEBUG deletes assert() together with the expression inside it, and half these checks wrap the call under test. assert(eos_net_connect(...) == 0) stopped connecting at all, the echo thread blocked in accept(), and the suite hung.

Ten call sites in the EOS_NET_POSIX section still used bare assert() — including that exact eos_net_connect line. They were dormant while assert() was a no-op. #104 added -UNDEBUG to the test targets, which is correct and is what makes the other 738 assertions in this suite real; it also made these ten live. The file never includes <assert.h>, so assert resolved to an implicit function declaration and went undefined at link.

Converted to CHECK() rather than adding the include: the include would make them real assertions again in Debug and silently-deleted ones in Release, which is the defect the macro was written to prevent.

Validation

Verified locally on macOS, all three configurations CI builds:


`cmake -B build/host -DCMAKE_BUILD_TYPE=Release`	builds — fails on master
`-DEOS_PRODUCT=medical`, `-DEOS_PRODUCT=aerospace`	both build
`-DEOS_BUILD_TESTS=ON -DEOS_PRODUCT=vbox_test` + ctest	34/34 passed, `test_net` among them

Negative control: the same default-configuration build against unpatched origin/master in a clean worktree reproduces the unknown type name errors verbatim, so the fix is doing the work and not the build directory.

Kartikey1306 · 2026-09-01T15:30:58Z

⚠️ #114 and #116 both add the #if EOS_ENABLE_NET guard to net/src/net_posix.c, and merging both breaks master — with no merge conflict to warn you.

Both PRs came from this account. #116 fixes the guard plus the test_net link error; #114 fixes the same guard plus adds a CMake test-registration check. The overlap is net/src/net_posix.c and tests/test_net.c.

Git merges them silently, because each adds its guard at a different point in the include block:

$ git merge fork/test/register-orphaned-suites   # onto #116
Auto-merging net/src/net_posix.c
Auto-merging tests/test_net.c
   (no conflict)

$ grep -c '^#if EOS_ENABLE_NET'            net/src/net_posix.c   ->  2
$ grep -c '^#endif /\* EOS_ENABLE_NET \*/' net/src/net_posix.c   ->  1

$ cmake -B build/host -DCMAKE_BUILD_TYPE=Release && cmake --build build/host
net/src/net_posix.c:35:2: error: unterminated conditional directive

Two opening guards, one closing. The default-configuration build fails exactly the way it does on master today, so merging both would put back the breakage both PRs exist to remove.

Suggested resolution: take #116 for the net_posix.c guard and the test_net.c fix, and drop those two files from #114, leaving it the tests/CMakeLists.txt registration guard and tests/unit/test_cmake_test_registration.py — which is the part of #114 that is genuinely its own and worth having. In either order, the second one merged then needs a rebase rather than a straight merge.

Related: I closed #113 in favour of #116 for the same overlap.

srpatcha

Review — eos#116 "fix: repair master — the default configuration and the test link"

head: d58bac4 author: Kartikey1306 ci: pass

Verdict: Both repairs have already landed on master by another route. Against
current master this PR reduces to a single line — removing an unused
#include <assert.h> from tests/test_net.c — and the title's claim to repair the
default configuration no longer holds. The remaining line is correct but does not need
this PR.

Findings

#	Severity	File:line	Finding	Recommended fix
1	Medium	PR title and body	"repair master — the default configuration and the test link" is no longer true at this head. `net/src/net_posix.c` is byte-identical to `origin/master`, and `master`'s default configuration builds clean. Per the brief, an unsupported claim in a PR body is itself the finding; here it would also mislead a maintainer into merging this ahead of work that actually is blocking.	Close the PR, or retitle it to what it now does — `chore(tests): drop the unused assert.h include from test_net.c` — and rewrite the body.
2	Low	`tests/test_net.c:5`	The one surviving change removes `#include <assert.h>`. That include was added to `master` one commit ago by #124, whose message reads "add the missing assert.h include that broke master". `tests/test_net.c` contains zero `assert()` calls at #124's parent and at #124 itself — the file uses its own `CHECK()` macro (`test_net.c:18`, 22 call sites). So the include is genuinely unused and removing it is right, but it reverts a commit whose stated reason does not match the file. Worth resolving with #124's author before it ping-pongs.	Land the removal, and correct the record on #124 so the next person does not re-add it.
3	Low	`net/src/net_posix.c:44` (on `master`, unchanged by this PR)	The `#if EOS_ENABLE_NET` guard fixes the symptom; the cause is that `CMakeLists.txt:201` adds `net_posix.c` to `eos_net` on `if(NOT CMAKE_CROSSCOMPILING)` — a platform condition — while whether the file has any contents is decided by `EOS_ENABLE_NET`, a C macro defaulting to `0` (`include/eos/eos_config.h:108`) and set to `1` only under `if(EOS_BUILD_TESTS AND NOT EOS_PRODUCT)` (`CMakeLists.txt:48-51`). The two conditions have no common source of truth, so they stay in agreement only by hand. Any other source added on a platform condition but bodied on a feature flag reproduces this exactly.	Promote `EOS_ENABLE_NET` to a real CMake `option()` that both drives `target_compile_definitions` and gates `target_sources`, so the file and its contents appear and disappear together by construction. Tracked as a design-doc proposal rather than a change to ask of this PR.
4	Low	`net/src/net.c:15`	Pre-existing warning surfaced while building this head: `warning: 'net_initialized' defined but not used [-Wunused-variable]`. With `EOS_ENABLE_NET=0` the variable's only users are compiled out.	Move the definition inside the `#if EOS_ENABLE_NET` block in `net.c`.

Evidence, against origin/master at feee272:

$ git diff origin/master refs/pull/116/head -- net/src/net_posix.c
                                        # no output — identical
$ git diff origin/master refs/pull/116/head -- tests/test_net.c
-#include <assert.h>
                                        # the whole remaining diff

$ grep -c '^#if EOS_ENABLE_NET' net/src/net_posix.c      # on master
1
$ cmake -B build/def && cmake --build build/def          # on master, default config
# exit 0

#124 and the include it added:

$ git show origin/master --stat
 tests/test_net.c | 1 +
$ git show feee272^:tests/test_net.c | grep -c 'assert('
0
$ git show feee272^:tests/test_net.c | grep -n 'define CHECK'
17:#define CHECK(cond) do { \

And the removal is safe:

$ cmake -B build/host -DEOS_BUILD_TESTS=ON && cmake --build build/host --target test_net
# exit 0, no error
$ ./build/host/tests/test_net
# exit 0

The assert → CHECK conversion this PR was credited with is the better of the two
available fixes — CHECK() survives -DNDEBUG, which is the whole point of #104 — and
it is already on master. Nothing here argues against the approach; it argues that the
work is done.

Architecture conformance

Conforms. Master design §21 places eos in Tier 1 - Foundation; net/ is the eNet
connectivity module named in §5 under Core Services → Connectivity, and it lives in the
owning repo rather than a separate one, which is correct under §21.1. No include, link
or manifest edge points up a tier: net_posix.c reaches only <netdb.h>,
<sys/socket.h> and eos/net.h. §5.1 holds.

Finding 3 is where the master design is silent rather than wrong, and I have appended a
proposal for it under .ai/autoreview/proposals/2026-09.md. §9.2 requires "one source
of truth for CLI, VS Code and EoStudio" and §10.1 gives components a Capabilities field,
but no section governs the relationship between build-system feature selection and the
C-level EOS_ENABLE_* macros that decide whether the selected code has any contents.
That gap is what broke the default configuration here.

Proposed changes

Confirm with #124's author that tests/test_net.c needs no assert.h — the
commands above settle it — then either close this PR or reduce it to that one line
with an accurate title.
Keep #114 and #115, which are stacked on this commit, moving. Because both carry
d58bac4, merging either one first brings this content along; closing this PR does
not lose anything.
Finding 4 is a one-line move and can ride with whichever PR next touches net/.

Do not merge this alongside a second PR that adds the same guard. That hazard was real
when #114 was a sibling; it is now resolved, because #114 is stacked on this commit and
carries exactly one #if EOS_ENABLE_NET / one #endif (verified below), so no
double-guard can arise from this pair:

$ git show refs/pull/114/head:net/src/net_posix.c | grep -c '^#if EOS_ENABLE_NET'
1
$ git show refs/pull/114/head:net/src/net_posix.c | grep -c '^#endif /\* EOS_ENABLE_NET \*/'
1

Not checked

I did not establish what actually motivated #124. I established only that
tests/test_net.c has no assert() call and builds without the include; if #124 was
fixing a different symptom, that reason is not visible in the file it changed.
I did not build any EOS_PRODUCT profile. EOS_ENABLE_NET is set to 1 only under
EOS_BUILD_TESTS AND NOT EOS_PRODUCT, so whether a product build gets eNet at all
depends on profile files I did not inspect. Finding 3 describes the mechanism, not the
consequence for products.
I did not run the cross-compile configurations. net_posix.c is excluded under
CMAKE_CROSSCOMPILING, so the guard should be inert there, but I verified only the
host default and host test builds.
CI is green at this head; I read the check list, not the job logs.
I did not review d58bac4 a second time in the #114 and #115 reports. It is the
shared base of all three.

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

…Boot Two things, and the second found the first. eos and eBoot each carry their own Ed25519 verifier. Different code, opposite return conventions — this one returns 1 to accept, eBoot's returns EOS_OK — doing the same job on the same wire format. When only eos rejected low-order public keys, nothing in either repo could notice: there is no shared build, and the two are far too different for a source diff to say anything. That divergence is why eBoot embeddedos-org#73 survived here after this side had already fixed it. tests/vectors/ed25519_contract_vectors.h is the part that can be shared: 76 vectors of pure data with a SHA-256 over them, byte-identical to eBoot's copy (eBoot embeddedos-org#89). Each side runs its own verifier and prints the digest, so a change to one copy that does not reach the other shows up as two different digests. On its first run it failed here, on three vectors eBoot passes. ed25519_verify() had only ref10's partial check, `signature[63] & 224`. That rejects any S with a bit set above 2^252, but L = 2^252 + 27742317777372353535851937790883648493 so every S in [L, 2^253) passes it while being non-canonical. Adding L to a valid S lands squarely in that band, and (R, S) and (R, S + L) both verified — signature malleability. Anything treating a signature as a unique identifier (deduplication, replay caches, logging by signature hash) sees two distinct signatures over one message and one key. RFC 8032 5.1.7 requires rejecting S outside [0, L). eBoot's core/ed25519_verify.c has done this all along via s_is_canonical(); this side did not. Added sc_is_canonical(), compared most-significant byte first against a public constant — S is public, so this leaks nothing. Verified: contract vectors, before the fix -> 3 of 76 wrong (all three S+L) contract vectors, after -> 76/76, 3 accepted, 73 refused test_crypto_ed25519_loworder -> 5/5, unchanged ctest -> 35/35 passed digest 1059febedae2b3e3dfaa1ef5b419fb37ed7f0d53b377a3250b53b95a546282a2 identical to eBoot embeddedos-org#89's copy Stacked on embeddedos-org#116; eos master does not build without it.

…removed `CI — eos` and `Python Unit Tests` are red on master (769191a), and both fail for one reason: these suites exist in tests/ but no add_executable() in tests/CMakeLists.txt builds them, so they never run: ['test_crypto_ed25519_loworder.c'] 3aa8644 (embeddedos-org#93, "remove duplicate test targets from tests/CMakeLists.txt") removed all four lines that build and register test_crypto_ed25519_loworder. It was not a duplicate — there was exactly one registration before that commit, and it took it. What stopped running is the suite that guards embeddedos-org#101: Ed25519 rejecting low-order public keys. A low-order key makes every term of the verification equation collapse to the identity regardless of the message, so a signature of all zeros verifies against any content at all. The fix is still in services/crypto/src/ed25519_verify.c and still correct; nothing has been checking it since embeddedos-org#93 merged, and nothing would have noticed if a later change had removed it too. Restored verbatim from 3aa8644^, in its original position before test_crypto_sha512. No other file changes. The guard that caught this is test_cmake_test_registration.py, added recently. It did exactly its job — this is the first thing it found. Verified: test_cmake_test_registration.py -> 5 passed (1 failed on master) pytest tests/ -> 15 passed (14 passed 1 failed on master) test_crypto_ed25519_loworder -> 5/5 tests passed ctest -> 39/39 passed (master builds 38)

Retitles and reduces this PR to what it still does, per the review. Both repairs it was opened for have landed on master by other routes: net/src/net_posix.c is byte-identical to origin/master, and master's default configuration builds clean. Against current master the branch's only real content was this one line -- and merging it as it stood would have *reverted* 646 lines that landed since, including embeddedos-org#120's pkg trust anchor, tests/test_pkg_trust_anchor.c and tests/unit/test_cmake_test_registration.py. Reset to master and re-applied the one surviving change rather than leave a stale branch that still looks mergeable. The include is genuinely unused: tests/test_net.c has zero assert() calls and uses its own CHECK() macro at 22 call sites. Worth correcting the record, per finding 2: embeddedos-org#124 added it one commit ago as "the missing assert.h include that broke master", and the file had no assert() at embeddedos-org#124's parent either. What fixed that link error was the assert() -> CHECK() conversion already on master; the include was never load-bearing, so removing it changes nothing except the next reader's understanding. Stacked on embeddedos-org#127, which restores the test_crypto_ed25519_loworder registration master is currently red on. Verified: pytest tests/ 15 passed ctest 39/39 PASS Refs embeddedos-org#116, embeddedos-org#124

srpatcha

Review — eos#116 "chore(tests): drop the unused assert.h include from test_net.c"

head: fafec15 author: Kartikey1306 ci: 2 required checks failing (pre-existing — see below)

Verdict: The diff is correct, minimal and worth landing: it removes an include with no remaining users, and it registers test_crypto_ed25519_loworder, a real security suite that has been sitting in tests/ unbuilt. Verified 39/39 against master's 38/38, and the extra test is the new one. The problem is the PR body, which describes an entirely different change from the one in the diff.

Findings

#	Severity	File:line	Finding	Recommended fix
1	Medium	PR body	The body describes work that is not in this diff and is already on master. It says this PR adds `#if EOS_ENABLE_NET` to `net/src/net_posix.c` — that guard is already there on `origin/master` at `net/src/net_posix.c:44` and `:216`, and this diff does not touch the file. It says ten bare `assert()` calls in the `EOS_NET_POSIX` section are converted to `CHECK()` — `tests/test_net.c` on this head contains zero occurrences of the string `assert` at all, and the diff's only change to it is deleting the `#include`. The "Validation" table then reports `34/34 passed` for a tree that no longer exists; this head runs 39. The title is accurate; the body is a stale artifact of a rebase. Per the brief, evidence attached to the wrong change is worse than no evidence, because it reads as verification.	Rewrite the body to the two things the diff does: delete a now-unused include that `#124` added, and register the orphaned `test_crypto_ed25519_loworder` suite. Re-run and paste `39/39`.
2	Low	existing comment, 2026-09-01T15:30:58Z	The pinned warning — "#114 and #116 both add the `#if EOS_ENABLE_NET` guard to `net/src/net_posix.c`, and merging both breaks master — with no merge conflict to warn you" — no longer applies to either PR. Neither head touches `net/src/net_posix.c`: this PR's changed files are `tests/CMakeLists.txt` and `tests/test_net.c`; #114's are `tests/CMakeLists.txt` and `tests/unit/test_cmake_test_registration.py`. The `unterminated conditional directive` scenario it demonstrates cannot occur from these two heads. A maintainer reading it will hold both PRs for a hazard that was fixed by the rebase.	Edit or delete the comment. It was a good catch when written; leaving it is now the misleading part.
3	Low	CI	Two required checks are red on this head: `Cross-compile ARM64 kernel` and `Full-stack integration summary`. They are not caused by this PR — the identical pair fails on all four open `eos` heads in this run, including #117, whose entire diff is seven lines of `.gitignore` plus deletion of committed build artifacts. So this is `master` being red, and this PR inheriting it. Naming it because the brief requires failing required checks to be named, not because it is yours.	Nothing to do here. It does mean this PR cannot be merged on a green tree until master's ARM64 cross-compile is fixed, which is worth knowing before you wait on it.

Architecture conformance

Conforms. eos is Tier 1 Foundation (§21) and both files are under tests/, which is the directory .ai/architect.md names in the reference layout for exactly this. No #include, link line or target_link_libraries entry points up a tier: the new registration links test_crypto_ed25519_loworder against eos_crypto, which is a core service inside the same repository, and adds no cross-repo edge. §5.1's dependency direction is untouched.

The registration is also the more valuable half of the diff, and it lands in the right place. tests/test_crypto_ed25519_loworder.c has been in the tree, unreferenced by tests/CMakeLists.txt, so a suite whose subject is rejecting keys that let an attacker forge a signature has never been compiled or run. That is the same defect class .ai/security.md calls out — "a security suite that silently collected nothing is a failed check reported as green" — one level up: not a suite that collected nothing, a suite that was never built. §28's evidence table puts "Implemented" at "code and functional tests"; an unbuilt test file does not meet it, and this diff is what does.

Proposed changes

Replace the PR body. The two sentences it needs are the ones in the title and the registration, plus the current ctest output.
Update or remove the 2026-09-01 cross-PR warning comment.
Nothing in the code. I would land the diff as it stands.

One thing worth doing in a follow-up, not here: test_crypto_ed25519_loworder.c was not the only orphan — this diff registers one file, and #114 adds tests/unit/test_cmake_test_registration.py to catch the class. Those two belong together, and #114's guard is what stops the next one going unnoticed.

Not checked

Not run: the two failing required checks. Cross-compile ARM64 kernel and Full-stack integration summary need a cross toolchain and the integration harness; I did not reproduce either, so I can state only that they fail on all four eos heads in this run and are therefore not attributable to this diff. I did not confirm the root cause on master.
Not verified: the body's claim about macOS. All my builds are Linux/GCC; the body says "Verified locally on macOS", and nothing here speaks to that platform.
Not verified: that test_crypto_ed25519_loworder is correct, only that it builds, runs and passes its own five cases. I did not check the low-order encodings it uses against the curve, nor confirm that eos_crypto rejects them for the right reason rather than incidentally.
Not checked: whether other test files under tests/ are also unregistered. That is #114's subject and I looked at this one file only.
Not checked: the -DEOS_PRODUCT=medical and -DEOS_PRODUCT=aerospace configurations named in the body. I built the default host configuration and vbox_test.

Evidence

Read-only git archive extractions of origin/master (feee272) and this head; the eos checkout was clean and untouched.

origin/master  cmake -B build/test -DCMAKE_BUILD_TYPE=Release -DEOS_BUILD_TESTS=ON \
                     -DEOS_PRODUCT=vbox_test && cmake --build            -> OK
               ctest --no-tests=error                                    -> 38/38 passed

this head      same configuration                                        -> OK
               ctest --no-tests=error                                    -> 39/39 passed
               cmake -B build/host -DCMAKE_BUILD_TYPE=Release            -> OK  (default config)

the +1:        ctest -R test_crypto_ed25519_loworder                     -> Passed
               ./tests/test_crypto_ed25519_loworder
                   test_all_zero_key_is_rejected                  [PASS]
                   test_identity_key_is_rejected                  [PASS]
                   test_every_low_order_key_is_rejected           [PASS]
                   test_rfc8032_vector_still_verifies             [PASS]
                   test_rfc8032_vector_rejects_a_tampered_message [PASS]
                   5/5 tests passed

safety of the deletion:
    tests/test_net.c @ this head : grep -c assert  -> 0
    tests/test_net.c @ master     : grep -n assert -> 5:#include <assert.h>   (the include only)

finding 1, against the body:
    net/src/net_posix.c @ master : line  44  #if EOS_ENABLE_NET
                                   line 216  #endif /* EOS_ENABLE_NET */   -> guard already merged
    net/src/net_posix.c          : not in this PR's changed-file list at all
    tests/test_crypto_ed25519_loworder.c exists on master, and
    tests/CMakeLists.txt @ master : grep -n ed25519 -> no match            -> orphaned until now

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

…Boot Two things, and the second found the first. eos and eBoot each carry their own Ed25519 verifier. Different code, opposite return conventions — this one returns 1 to accept, eBoot's returns EOS_OK — doing the same job on the same wire format. When only eos rejected low-order public keys, nothing in either repo could notice: there is no shared build, and the two are far too different for a source diff to say anything. That divergence is why eBoot embeddedos-org#73 survived here after this side had already fixed it. tests/vectors/ed25519_contract_vectors.h is the part that can be shared: 76 vectors of pure data with a SHA-256 over them, byte-identical to eBoot's copy (eBoot embeddedos-org#89). Each side runs its own verifier and prints the digest, so a change to one copy that does not reach the other shows up as two different digests. On its first run it failed here, on three vectors eBoot passes. ed25519_verify() had only ref10's partial check, `signature[63] & 224`. That rejects any S with a bit set above 2^252, but L = 2^252 + 27742317777372353535851937790883648493 so every S in [L, 2^253) passes it while being non-canonical. Adding L to a valid S lands squarely in that band, and (R, S) and (R, S + L) both verified — signature malleability. Anything treating a signature as a unique identifier (deduplication, replay caches, logging by signature hash) sees two distinct signatures over one message and one key. RFC 8032 5.1.7 requires rejecting S outside [0, L). eBoot's core/ed25519_verify.c has done this all along via s_is_canonical(); this side did not. Added sc_is_canonical(), compared most-significant byte first against a public constant — S is public, so this leaks nothing. Verified: contract vectors, before the fix -> 3 of 76 wrong (all three S+L) contract vectors, after -> 76/76, 3 accepted, 73 refused test_crypto_ed25519_loworder -> 5/5, unchanged ctest -> 35/35 passed digest 1059febedae2b3e3dfaa1ef5b419fb37ed7f0d53b377a3250b53b95a546282a2 identical to eBoot embeddedos-org#89's copy Stacked on embeddedos-org#116; eos master does not build without it.

Every open eos pull request is currently red on `EoS Full-Stack Simulation`, and none of them caused it. The kernel job checks out embeddedos-org/eBoot at `ref: master`, and eBoot master does not compile: include/eos_image.h:135 and :142 static-assert offsetof(eos_image_header_t, reserved) for a member that became tlv_len + tlv_hash, and core/ed25519_verify.c redefines point_is_identity -- two pairs of PRs that each merged clean and broke the build together. eBoot embeddedos-org#94 repairs it and waits on review; until it lands, no change to any eos PR can make this job pass, and after it lands the next broken eBoot master does the same thing again. A floating `ref: master` makes every eos PR's CI depend on the moment-to- moment state of another repository's default branch. Pinning the two compiled-against checkouts makes the job a function of this repository plus two recorded SHAs -- red means the PR broke something, green means it did not, and a cross-repo bump is a reviewable one-line diff with CI on it. EBOOT_COMMIT a172a6d6e1e6 the newest eBoot master ancestor that compiles; found by walking master back and building each candidate. Its child 8a015b2 already carries the ed25519 redefinition. EBUILD_COMMIT e5d8052f3e2c ebuild's current master, frozen as-is. Both of its checkouts feed steps that are commented out (eFab does not exist), so this pin only removes the float. The middleware matrix (10 repos) still floats; it runs those repos' own tests rather than compiling eos against them, and 10 more pins deserve their own decision. When eBoot embeddedos-org#94 merges, bump EBOOT_COMMIT to the merge SHA. Verified before pushing, master vs pin under the job's own configuration (-DEBLDR_BOARD=qemu_arm64 -DEBLDR_VERIFY_STAGE1=OFF -DCMAKE_BUILD_TYPE=Release, cross-compile mode): eBoot@master fails at the eos_image.h asserts, the exact error in the job log; eBoot@a172a6d builds every target, rc=0. Both pinned SHAs verified reachable from their repos' master. Workflow parses; the three `ref:` edits and the env block are the whole diff. This PR's own simulation run is the live test: it uses this branch's workflow and should go green while every master-based PR stays red. Carried onto this branch from embeddedos-org#135 (001128d) so this PR's own simulation job runs against a buildable eBoot: each pull request executes its own copy of the workflow, so the fix greens a PR only once its branch contains it. Byte-identical to embeddedos-org#135's commit -- git drops it as already-upstream the moment embeddedos-org#135 merges. The pin itself is embeddedos-org#135's to review.

srpatcha

Review — eos#116 "chore(tests): drop the unused assert.h include from test_net.c"

head: 2a32e3a author: Kartikey1306 ci: 20 pass, 3 pending, 0 failing (see the note on Cross-compile ARM Cortex-M4 below)

Verdict: The two changes that match the title are still correct and still worth landing — verified at this head: default host configuration builds, and ctest --no-tests=error gives 39/39 against master's 38, the extra being the newly registered test_crypto_ed25519_loworder. tests/test_net.c contains zero occurrences of assert at this head, so deleting the include is safe. What changed since fafec150 is that the PR grew a third change which is a byte-identical copy of another open PR's, and the body still describes none of the three.

Findings

#	Severity	File:line	Finding	Recommended fix
1	Medium	`.github/workflows/eos-simulation.yml:11-19`, `:41`, `:48`, `:288`	`2a32e3a` ("ci: pin the eBoot and ebuild checkouts instead of floating on master") is the same change as #135, which carries the same commit title and is open and under review. Verified: `git diff <this head> <#135 head> -- .github/workflows/eos-simulation.yml` is empty — the `env:` block, the comment, and all three `ref:` substitutions are identical, byte for byte. #135 has since added `f34edb3` ("ci: make the pin visible when it goes stale"), which this copy does not have. Identical hunks merge without conflict, so nothing breaks; the cost is that a cross-repository CI-dependency change is now sitting in a PR titled "drop the unused assert.h include", where a maintainer reviewing the title will not look for it and a maintainer reviewing #135 will not know it is duplicated. That is the same overlap hazard you documented yourself for #114/#116 on 2026-09-01.	Drop `2a32e3a` from this branch. #135 is the right home: it is scoped to the pin, it is the head that has the staleness guard, and it has already been reviewed twice. If the pin is needed here to get a green run, say so on the thread rather than carrying the commit. The pin's substance — the missing staleness signal, and the ten-repo middleware matrix still on `ref: master` at `:248` — is already reviewed on #135 and is not re-raised here.
2	Medium	PR body	Unresolved from the `fafec150` review, and now wrong about three things instead of two. The body says this PR adds `#if EOS_ENABLE_NET` to `net/src/net_posix.c` — the guard is already on `origin/master` (`feee272`) and this diff does not touch the file. It says ten bare `assert()` calls are converted to `CHECK()` — `tests/test_net.c` at this head has no `assert` in it at all and the diff's only change to it is the deleted `#include`. Its "Validation" table reports `34/34`; this head runs 39/39. And it says nothing whatever about the `eos-simulation.yml` pin, which is now the largest part of the diff. Per the brief, a PR body asserting "verified" against a tree that no longer exists is itself the finding, because it reads as evidence for the change in front of you.	Rewrite the body to the diff: delete an include that `#124` made redundant, register the orphaned `test_crypto_ed25519_loworder` suite, and (if `2a32e3a` stays) the pin. Paste the current `39/39`.
3	Low	existing comment, 2026-09-01T15:30:58Z	Also unresolved from the previous review. The pinned warning — "#114 and #116 both add the `#if EOS_ENABLE_NET` guard to `net/src/net_posix.c`, and merging both breaks master" — is true of neither head any more: this PR's files are `.github/workflows/eos-simulation.yml`, `tests/CMakeLists.txt` and `tests/test_net.c`; #114's are `tests/CMakeLists.txt` and `tests/unit/test_cmake_test_registration.py`. Neither touches `net/src/net_posix.c`, so the `unterminated conditional directive` it demonstrates cannot happen. It was a good catch when written and is now the thing holding both PRs.	Edit or delete it. The real overlap between these two heads today is `tests/CMakeLists.txt`, which is a different and much smaller problem — see #114's review.

Architecture conformance

Conforms. eos is Tier 1 — Foundation (§21) and all three files are infrastructure: tests/ is the directory .ai/architect.md names in the reference layout, and .github/workflows/ is Infrastructure per §21. No #include, link line or target_link_libraries entry points up a tier — the new registration links test_crypto_ed25519_loworder against eos_crypto, a core service in the same repository, adding no cross-repo edge, so §5.1's runtime direction is untouched.

The pin in 2a32e3a is a CI-time dependency from eos onto eBoot and ebuild, both Tier 1, so it points sideways rather than up. §5.1 governs runtime dependencies and is silent on this class; that gap and this exact pin are already recorded in .ai/autoreview/proposals/2026-09.md (the 2026-09-03 entry on build- and CI-time dependencies, and the 2026-09-04 addendum triggered by #135, which analyses this same SHA). Nothing new to propose from this PR. I did confirm the pin is a real, reachable commit: a172a6d is fix(ed25519): reject public keys outside the prime-order subgroup (#86) and is an ancestor of eBoot's origin/master; e5d8052 is ebuild's current master merge.

The registration remains the more valuable half of the diff and lands in the right place. tests/test_crypto_ed25519_loworder.c was in the tree unreferenced by tests/CMakeLists.txt, so a suite whose subject is rejecting keys that let an attacker forge a signature had never been compiled. That is the defect class .ai/security.md names — "a security suite that silently collected nothing is a failed check reported as green" — one level up, and §28's "Implemented = code and functional tests" is not met by a test file nothing builds.

Proposed changes

Drop 2a32e3a; let #135 carry the pin.
Rewrite the body against the actual diff and paste 39/39.
Edit or delete the 2026-09-01 cross-PR comment.
Nothing in the C or CMake. I would land the remaining two-file diff as it stands.

Not checked

Analyze (C/C++), Cross-compile ARM Cortex-M4 and QEMU ARM64 simulation (13 TCs) were still pending when this bundle was built, so I cannot report their outcome. Nothing was failing. Worth recording that Cross-compile ARM64 kernel — red on this PR at fafec150 and on every other open eos head in the previous run — is green at this head, which is the pin doing its job.
Cross-compile ARM Cortex-M4 appears twice in checks.txt, once pass and once pending. Two workflows declare the same check name: .github/workflows/build.yml:74 and .github/workflows/ci.yml:117. Not introduced by this PR, and not a finding against it, but it means "is that check green" has no single answer on any eos PR — the same defect ebuild#104 is fixing in ebuild, present here. Naming it because the brief requires CI state to be stated precisely.
The body's macOS claim: NOT VERIFIED. All my builds are Linux/GCC.
-DEOS_PRODUCT=medical and -DEOS_PRODUCT=aerospace: NOT BUILT. I built the default host configuration and vbox_test only.
test_crypto_ed25519_loworder's correctness: NOT VERIFIED, only that it builds, runs and passes. I did not check its low-order encodings against the curve, nor that eos_crypto rejects them for the right reason.
Whether other test files under tests/ remain unregistered: NOT CHECKED here. That is #114's subject.
The pinned eBoot tree was not built. I confirmed a172a6d exists and is on eBoot's master; I did not compile eos's kernel job against it, so "the pin makes the job green" rests on checks.txt, not on anything I ran.

Evidence

Read-only git archive extraction of 2a32e3a; the eos checkout was clean and was not modified.

this head   cmake -B build/test -DCMAKE_BUILD_TYPE=Release -DEOS_BUILD_TESTS=ON \
                  -DEOS_PRODUCT=vbox_test && cmake --build          -> OK
            ctest --no-tests=error                                  -> 39/39 passed
            cmake -B build/host -DCMAKE_BUILD_TYPE=Release          -> OK  (default config)

deletion is safe:
            tests/test_net.c @ 2a32e3a       grep -n assert  -> no match
            tests/test_net.c @ origin/master grep -n assert  -> 5:#include <assert.h>   (only)

finding 1:
            git diff 2a32e3a <#135 head> -- .github/workflows/eos-simulation.yml  -> empty
            #135 head is f34edb3, one commit ahead (the staleness guard this copy lacks)

the pin:
            eBoot  a172a6d  "fix(ed25519): reject public keys outside the prime-order
                             subgroup (#86)"  — ancestor of eBoot origin/master
            ebuild e5d8052  "Merge pull request #102"  — ebuild origin/master

Automated architecture review of 2a32e3a919c1 — 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.

…Boot (#122) * fix(build): restore the low-order Ed25519 suite that #93 removed `CI — eos` and `Python Unit Tests` are red on master (769191a), and both fail for one reason: these suites exist in tests/ but no add_executable() in tests/CMakeLists.txt builds them, so they never run: ['test_crypto_ed25519_loworder.c'] 3aa8644 (#93, "remove duplicate test targets from tests/CMakeLists.txt") removed all four lines that build and register test_crypto_ed25519_loworder. It was not a duplicate — there was exactly one registration before that commit, and it took it. What stopped running is the suite that guards #101: Ed25519 rejecting low-order public keys. A low-order key makes every term of the verification equation collapse to the identity regardless of the message, so a signature of all zeros verifies against any content at all. The fix is still in services/crypto/src/ed25519_verify.c and still correct; nothing has been checking it since #93 merged, and nothing would have noticed if a later change had removed it too. Restored verbatim from 3aa8644^, in its original position before test_crypto_sha512. No other file changes. The guard that caught this is test_cmake_test_registration.py, added recently. It did exactly its job — this is the first thing it found. Verified: test_cmake_test_registration.py -> 5 passed (1 failed on master) pytest tests/ -> 15 passed (14 passed 1 failed on master) test_crypto_ed25519_loworder -> 5/5 tests passed ctest -> 39/39 passed (master builds 38) * fix(ed25519): reject a non-canonical S, and share the contract with eBoot Two things, and the second found the first. eos and eBoot each carry their own Ed25519 verifier. Different code, opposite return conventions — this one returns 1 to accept, eBoot's returns EOS_OK — doing the same job on the same wire format. When only eos rejected low-order public keys, nothing in either repo could notice: there is no shared build, and the two are far too different for a source diff to say anything. That divergence is why eBoot #73 survived here after this side had already fixed it. tests/vectors/ed25519_contract_vectors.h is the part that can be shared: 76 vectors of pure data with a SHA-256 over them, byte-identical to eBoot's copy (eBoot #89). Each side runs its own verifier and prints the digest, so a change to one copy that does not reach the other shows up as two different digests. On its first run it failed here, on three vectors eBoot passes. ed25519_verify() had only ref10's partial check, `signature[63] & 224`. That rejects any S with a bit set above 2^252, but L = 2^252 + 27742317777372353535851937790883648493 so every S in [L, 2^253) passes it while being non-canonical. Adding L to a valid S lands squarely in that band, and (R, S) and (R, S + L) both verified — signature malleability. Anything treating a signature as a unique identifier (deduplication, replay caches, logging by signature hash) sees two distinct signatures over one message and one key. RFC 8032 5.1.7 requires rejecting S outside [0, L). eBoot's core/ed25519_verify.c has done this all along via s_is_canonical(); this side did not. Added sc_is_canonical(), compared most-significant byte first against a public constant — S is public, so this leaks nothing. Verified: contract vectors, before the fix -> 3 of 76 wrong (all three S+L) contract vectors, after -> 76/76, 3 accepted, 73 refused test_crypto_ed25519_loworder -> 5/5, unchanged ctest -> 35/35 passed digest 1059febedae2b3e3dfaa1ef5b419fb37ed7f0d53b377a3250b53b95a546282a2 identical to eBoot #89's copy Stacked on #116; eos master does not build without it. * build(tests): generate the Ed25519 contract vectors, do not commit them tests/vectors/ed25519_contract_vectors.h is 54 KB of generated data, and the identical file was checked in to BOTH eos and eBoot, along with the generator that produces it -- 61 KB duplicated verbatim across two repositories and kept in step by hand. The generator is pure stdlib and deterministic, so each repo can produce the header into its own build tree instead. tools/gen_ed25519_contract_vectors.py grows an -o/--output flag (stdout stays the default, so the regeneration command in its docstring still works), and tests/CMakeLists.txt drives it from add_custom_command. Only the 6.8 KB generator is now carried twice, and it is source rather than generated output. Verified: the generated header is byte-identical to the file it replaces; both repos still report digest 1059febedae2b3e3dfaa1ef5b419fb37ed7f0d53b377a3250b53b95a546282a2, which is the thing that actually holds the two verifiers to one contract. Suites pass (eos 35/35, eBoot 22/22). Breaking the generator fails the build rather than silently reusing a stale header. The cross-compile legs configure with tests OFF so find_package(Python3) is never reached there; confirmed by configuring under a cross toolchain. * test(ed25519): assert the contract digest, and test the [0, L) boundary directly Answers the review on #122. The malleability fix itself was found correct; these are the findings around it. Finding 1 (High) -- the digest was printed and never compared, so the shared contract could not fail. EOS_ED25519_CONTRACT_DIGEST came from the header this repo's own generator had just written, a hash taken over its own output, so editing the generator here and not in eBoot produced a self-consistent corpus with a new digest, a green test, and a divergence visible only to a human reading two CI logs. Now pinned as a literal in committed source, together with the vector count and accept count, which are generated for the same reason and were equally unfalsifiable. The eBoot twin (#89) pins the identical value. Finding 4 (Medium) -- sc_is_canonical()'s accept side had no committed test. Through the public API the boundary is unreachable: S == L and a failed group equation are both simply 0, so an off-by-one refusing a valid S = L - 1 would satisfy every vector in the corpus and reject real signatures only in the field. It is a pure function of 32 bytes, so tests/test_ed25519_canonical_s.c tests it directly -- 0, 1, L-1, L, L+1, 2^252, 2^252+2^251 and all-ones -- with no signing key and no `cryptography` module. ed25519_verify.c is compiled a second time with EOS_ED25519_TEST_HOOKS to give the function external linkage; the shipped build is unchanged. This is the case the finding was about, reproduced. With `S == L` accepted: test_ed25519_canonical_s [FAIL] at the S = L case test_ed25519_contract [PASS] all 76 vectors The corpus cannot see it. The direct test can. Also corrected while writing it: 2^252 is BELOW L (L = 2^252 + 2.77e37), so it is in range. The value that is both non-canonical and passed by ref10's `signature[63] & 224` test is 2^252 + 2^251 -- 0x18 & 224 == 0. Both are now asserted, the second alongside the mask check that shows why the partial test misses it. Finding 2 (Medium) -- tests/vectors/ed25519_contract_vectors.h is now committed rather than generated. CMAKE_CURRENT_SOURCE_DIR preceded CMAKE_CURRENT_BINARY_DIR on the include path, so a stale source-tree copy would silently win over the generated one -- and the PR body's own re-derivation command wrote to exactly that path. Committing removes the shadow, makes the corpus auditable from a tag, and drops find_package(Python3 ... REQUIRED) from a pure-C suite. NOT fixed here, and stated rather than left implied: finding 3, the corpus omitting the y >= p (non-canonical public key) family. That is the class most likely to diverge between the two verifiers and it is worth adding -- but adding vectors changes the digest, which both repos now pin, so it has to land in eos and eBoot together in one coordinated change. That is the mechanism working as intended, and it is a follow-up rather than something to slip into this PR. Verified: ctest 41/41 PASS test_ed25519_contract 76 vectors, 3 accept, 73 refuse test_ed25519_canonical_s PASS rebased onto master; conflicts in tests/CMakeLists.txt and services/crypto/src/ed25519_verify.c resolved additively -- master's ed25519_public_key_is_usable() (#120) and this branch's sc_is_canonical() both kept. Refs #122 * test(ed25519): hash the vectors, and drop the linkage macro CodeQL flagged Answers the second round on #122. Both findings are defects in my previous commit. The digest pin did not cover the corpus. EOS_ED25519_CONTRACT_DIGEST is a #define inside the generated header; EOS_ED25519_CONTRACT_EXPECTED is a #define in the driver. The test compared two string literals and never hashed a vector, so editing a committed vector changed the data and left the value it was compared against untouched. Raised on the eBoot twin (#89) and stated there to apply here too; it did. contract_digest() now recomputes SHA-256 over the vectors at run time in the generator's serialisation -- public_key[32] || signature[64] || message[message_len] || (uint8_t)expect_accept -- via eos_sha256_* from eos/crypto.h. It compares the result to the pinned literal and to the header's own claim, so a corpus and a header from different generator runs are caught. Verified with the probe from the eBoot review. Changing one byte of the order-1 identity public key: [FAIL] corpus digest changed expected 1059febe...282a2 got 55370deb...cae9 exit 1 That `got` value is byte-identical to the one eBoot's driver produces for the same edit, which is independent confirmation that the two committed corpora really are the same file. Worth recording because it nearly cost me the finding: the first probe run reported PASS. The build had not picked up the header change, so the binary was stale. `touch` on the driver and a rebuild gave the failure above. A probe that does not rebuild proves nothing -- check that the link step ran. CodeQL alert 405, "Static function sc_is_canonical is unreachable" -- caused by the EOS_ED25519_TEST_HOOKS macro I added, which gave the symbol a linkage that varied by build. Removed: the function is static again, and tests/test_ed25519_canonical_s.c includes the translation unit to reach it, which is the ordinary way to test a static and needs no conditional compilation in a crypto source file. eos_crypto is still linked for the ge_*/sha512 helpers the included unit calls -- a static archive contributes only objects needed for undefined symbols, and ed25519_verify.c.o is not one, because the test object already defines everything in it. Verified: ctest 41/41 PASS test_ed25519_canonical_s PASS, links with no duplicate symbols test_ed25519_contract 76 vectors, 3 accept, 73 refuse byte-flip probe FAILS (after a real rebuild) off-by-one probe, `S == L` accepted: test_ed25519_canonical_s FAILS test_ed25519_contract still exit 0 so the direct boundary test still discriminates where the corpus cannot. Refs #122 * docs(ed25519): make the two copies actually byte-identical, drop stale comments Addresses srpatcha's review findings 1 and 3 on #122. Text and comments only; no behaviour change, and the 76 vectors and their digest are untouched. Finding 1 -- the body claimed the corpus was byte-identical to eBoot#89's copy and it was not: the two generators had drifted in their docstrings and in the header comment they emit, so the files differed by three comment lines while all 76 vectors and the digest matched. Adopted eBoot#89's wording verbatim -- it is the more accurate description, since each side recomputes the digest and compares it to a pinned literal rather than merely printing it -- and regenerated. Both repos' tools/gen_ed25519_contract_vectors.py and tests/vectors/ed25519_contract_vectors.h are now byte-identical: sha256 489b1fc0662187841c58e12c650ca1a25a974e4fafe4d4318f4fad62b7e4b5cf which is the value the review measured for eBoot's copy, so the spot-check the finding describes -- hash both files -- now passes. Finding 3 -- three comment blocks described an approach that was abandoned, two of them contradicting the code beside them: (a) ed25519_verify.c opened "Linked, not static, when the test hook is on" six lines above "It stays static", and the function is static. (b) tests/CMakeLists.txt described compiling ed25519_verify.c a second time with EOS_ED25519_TEST_HOOKS. That macro occurred exactly once in the tree -- in that sentence. The next line already describes what the test really does (#include the translation unit). (c) the paragraph saying the vector table "is produced into the build tree rather than committed" sat directly above one saying it is committed. It is committed; the generate-at-build-time paragraph was mine, left behind when that approach was reverted, and it is the half that was false. In each case the surviving text already explained the real design, so this is subtraction. Verified: cmake+ctest 41/41 (the count finding 4 asks for, not the 35/35 still in the body), test_ed25519_contract reports digest 1059feb... over 76 vectors with 3 accepted and 0 wrong, and test_ed25519_canonical_s still passes. --------- Co-authored-by: Srikanth Patchava <srikanth.patchava@outlook.com>

Kartikey1306 requested review from hshanmug12 and srpatcha as code owners September 1, 2026 15:05

This was referenced Sep 1, 2026

chore: untrack the sanitizer build trees committed in #110 #117

Merged

fix(linux_security): close the gaps in the shell-path guard, and test it #119

Merged

fix(net): guard net_posix.c, which broke every non-test build #113

Closed

srpatcha reviewed Sep 3, 2026

View reviewed changes

srpatcha mentioned this pull request Sep 3, 2026

fix(tests): restore POSIX network tests and kernel regression coverage #123

Merged

Kartikey1306 force-pushed the fix/master-net-build-and-test-link branch from d58bac4 to 2a26424 Compare September 3, 2026 10:05

Kartikey1306 changed the title ~~fix: repair master — the default configuration and the test link~~ chore(tests): drop the unused assert.h include from test_net.c Sep 3, 2026

Kartikey1306 added 2 commits September 3, 2026 15:35

Kartikey1306 force-pushed the fix/master-net-build-and-test-link branch from 2a26424 to fafec15 Compare September 3, 2026 10:06

srpatcha reviewed Sep 3, 2026

View reviewed changes

Kartikey1306 mentioned this pull request Sep 4, 2026

ci: pin the eBoot and ebuild checkouts instead of floating on master #135

Merged

srpatcha reviewed Sep 4, 2026

View reviewed changes

srpatcha merged commit 816e3db into embeddedos-org:master Sep 8, 2026
28 checks passed

srpatcha mentioned this pull request Sep 14, 2026

ci: grade only open pull requests against the linked-issue policy #154

Merged

Navigation Menu

Repository navigation

chore(tests): drop the unused assert.h include from test_net.c - #116

chore(tests): drop the unused assert.h include from test_net.c#116
srpatcha merged 3 commits into
embeddedos-org:masterfrom
Kartikey1306:fix/master-net-build-and-test-link

Kartikey1306 commented Sep 1, 2026 •

edited

Loading

Uh oh!

Kartikey1306 commented Sep 1, 2026

Uh oh!

srpatcha left a comment

Uh oh!

srpatcha left a comment

Uh oh!

srpatcha left a comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

Kartikey1306 commented Sep 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

1. The default configuration does not compile

2. test_net does not link

Validation

Uh oh!

Kartikey1306 commented Sep 1, 2026

Uh oh!

srpatcha left a comment

Choose a reason for hiding this comment

Review — eos#116 "fix: repair master — the default configuration and the test link"

Findings

Architecture conformance

Proposed changes

Not checked

Uh oh!

srpatcha left a comment

Choose a reason for hiding this comment

Review — eos#116 "chore(tests): drop the unused assert.h include from test_net.c"

Findings

Architecture conformance

Proposed changes

Not checked

Evidence

Uh oh!

srpatcha left a comment

Choose a reason for hiding this comment

Review — eos#116 "chore(tests): drop the unused assert.h include from test_net.c"

Findings

Architecture conformance

Proposed changes

Not checked

Evidence

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Kartikey1306 commented Sep 1, 2026 •

edited

Loading

2. `test_net` does not link