Skip to content

ci: opt the eBoot builds into the development key, as CI builds that are not releases - #152

Open
Kartikey1306 wants to merge 10 commits into
embeddedos-org:masterfrom
Kartikey1306:ci/eboot-builds-opt-into-dev-key
Open

Kartikey1306 wants to merge 10 commits into
embeddedos-org:masterfrom
Kartikey1306:ci/eboot-builds-opt-into-dev-key

Conversation

@Kartikey1306

@Kartikey1306 Kartikey1306 commented Sep 14, 2026 •

Copy link
Copy Markdown
Contributor

Stacked on #144 (master's Python guards are red without it). Review 9569226..b55c147: one commit, two workflow files, +11 lines.

Scope widened at cd95cf7, on review: the same gate breaks the eight documented cmake -B … -DEBLDR_BOARD=<board> commands in this repository, so they are fixed here too — seven development snippets get -DCMAKE_BUILD_TYPE=Debug, and the one deployment snippet (DEPLOYMENT.md:133) gets -DEBLDR_PRODUCTION_KEY=<64 hex characters: your Ed25519 public key>, which is what a device image needs. Every edit is valid on today's eBoot master (measured, configure-only, against master 682d005 and eBoot#122 1cd1265), so this still lands before eBoot#122. three-way-alignment.md:209 is the same line in #151's rewrite, so #151/#152 rebase cleanly in either order.

Problem (eos #150)

eos-simulation.yml and upstream-drift.yml build eBoot as a Release build of qemu_arm64. embeddedos-org/eBoot#122 (the fix for eBoot#120) makes exactly that configure fail closed unless the build states that it is not a release: a release-shaped build must carry a production trust anchor, because the development key's secret is public. Once it lands, Upstream drift — which builds eBoot master nightly — goes red at configure, and EoS Full-Stack Simulation follows when EBOOT_COMMIT is next bumped.

Change

Both configures pass -DEBLDR_ALLOW_DEV_KEY=ON, with a comment on the simulation step saying why: these builds exist to run the simulation and to see whether master still builds, and nothing they produce reaches a device — which is precisely the case that option exists for.

Safe to land first

On the currently pinned eBoot (a172a6d), which predates the option, the flag produces CMake's "Manually-specified variables were not used by the project" warning, not an error. Verified locally: configuring that eBoot revision with the flag exits 0. So this can merge before eBoot#122, and EBOOT_COMMIT can be bumped once eBoot#115 has merged (the note in #144).

Verification

YAML parses; pytest tests/unit 43 passed on this branch (test_ci_gate.py reads every workflow).

Closing issue

Fixes #150

Nine PRs were merged into master within minutes on 09-08, each on the base it
was written against. Master has not compiled since, and the Python guards
that would have named the rest never ran because the C build failed first.

Compile (every C job, CodeQL, and the ARM64 kernel in the simulation):
- services/linux/src/linux_security.c: embeddedos-org#119 and embeddedos-org#132 each added an #else to
  the same #ifndef _WIN32 in eos_busybox_install_to_rootfs(), so master has
  "#else after #else". The embeddedos-org#132 arm (`(void)bb;`) is the one removed -- the
  embeddedos-org#119 arm already uses bb and reports the unsupported platform.

Guards from embeddedos-org#121 / embeddedos-org#93 that later merges walked back:
- ci.yml: embeddedos-org#132 added windows-test after embeddedos-org#121's gate; the gate did not wait
  for it, so "CI Gate" could be green with the MSVC leg red.
- test_ci_gate.py: embeddedos-org#129 gave eosim-sanity.yml and simulation-test.yml a
  path-filtered pull_request trigger (they test their own edits); a
  path-filtered check cannot be required, so both are recorded in
  NOT_REQUIRED with the book-build.yml reason.
- tests/CMakeLists.txt: test_linux_security_paths (embeddedos-org#119) and test_pkg_fetch
  (embeddedos-org#115) had no add_executable(). embeddedos-org#115's replay replaced embeddedos-org#119's block with
  its own, and embeddedos-org#118's replay replaced that; two suites compiled against
  nothing. Both registered again. 41 -> 43 suites.
- tests/test_kernel.c: four tests from embeddedos-org#130 and embeddedos-org#131 were defined and never
  called -- their RUN() lines did not survive the replay of main().

And the one that was not bookkeeping:
- kernel/src/task.c: embeddedos-org#130 was merged after embeddedos-org#131 from a base that predates it,
  and its copy of task.c replaced embeddedos-org#131's. embeddedos-org#131 had also flattened 574 CRLF
  line endings, so its 1166-line diff hid a 39/21 change and the replay took
  embeddedos-org#130's side wholesale. Master kept embeddedos-org#131's tests and lost its kernel: the
  idle task could be deleted and suspended, a half-initialised TCB was
  published to the scheduler before its stack existed, and eos_schedule()
  pointed g_current_sp at the outgoing task. test_idle_task_is_permanent
  fails on master the moment it is called. embeddedos-org#131's task.c diff re-applied
  on top of embeddedos-org#130's; the result is embeddedos-org#131's file plus embeddedos-org#130's wake_armed hunk
  and nothing else (verified by diff against 3a00bd9).

Verified locally (macOS, clang): Release build clean, 43/43 ctest; the
README default configuration builds; 47/47 pytest.

Not in this PR: the nightly "Upstream drift" job builds eBoot at master and
eBoot master is broken separately (its own repair PR); bump EBOOT_COMMIT in
eos-simulation.yml once that lands.
Re-registering it put it in front of the MSVC leg (embeddedos-org#132) for the first time,
where <unistd.h> does not exist. The suite probes with access(), open(O_CREAT)
and rmdir to prove refused inputs never reach a shell; on Windows the
library's own #else arm refuses every install/build call before
is_path_safe() matters, so there is nothing for it to test there.
… gives 1 MB

Re-registering the POSIX-only suite let the MSVC leg reach ctest for the
first time, and test_firmware died there with SEGFAULT: five tests declare
a stack EosConfig (sizeof 3,953,236 on this ABI), which the 8 MB default
stack on Linux and macOS absorbs and MSVC's 1 MB does not. test_config.c
already keeps its EosConfig static for the same reason; the same here.
kernel.h declared eos_task_delete() and eos_task_suspend() without saying
what handle 0 is. It is the idle task, it is permanent, and both calls
return EOS_KERN_INVALID for it -- as they do for an out-of-range handle or
an empty slot. The doc comments now say so, in the header's Doxygen style.

eos_schedule() has an early return for "no runnable task" that is
unreachable by construction: slot 0 cannot be deleted or suspended, the
idle task only spins or waits for an interrupt and so never blocks, and
find_next_task() can select it because of the "best < 0 ||" term. The
comment records that reasoning, and that the return is self-healing if it
were ever taken: the outgoing task stays READY and the next eos_schedule()
marks it RUNNING.

task.c also carried two stray blank lines at EOF from the re-apply of
embeddedos-org#131; with them gone, the diff against 3a00bd9 is embeddedos-org#130's wake_armed hunk
plus this comment and nothing else.
…rootfs

is_path_safe() refuses NULL, but eos_busybox_install_to_rootfs() read
bb->source_dir to hand it to the predicate, so a NULL bb reached the
dereference before the guard could see it: a crash rather than the -1
every other refusal in this file produces. Both arguments are now checked
first, before either is read.

test_linux_security_paths gains a case for it: (NULL, dir) and (&bb, NULL)
both return -1, and the directory gets no /init. With the guard removed
and the target rebuilt, the suite dies with SIGSEGV (exit 139) on the
first call; with it restored, the suite passes.
Records under Unreleased what embeddedos-org#144 did and why: the "#else after #else"
that stopped master compiling after the 09-08 batch merge, the guards the
replays walked back (two suites re-registered, four test_kernel tests
wired into main(), windows-test in the CI gate, the two path-filtered
simulation workflows classified), and the NULL guard in
eos_busybox_install_to_rootfs().

The scheduler change gets its own Changed entry because it is a contract:
eos_task_delete(0) and eos_task_suspend(0) return EOS_KERN_INVALID again,
as embeddedos-org#131 specified before embeddedos-org#130's replay replaced its task.c. Changed sits
above Fixed, the order the older releases in this file use.
…end checks

The eos_schedule() comment claimed the early return was self-healing. It is
not: find_next_task() scans every slot including the outgoing one, so
next < 0 means nothing is READY, and the return leaves the state exactly as
the canary check left it. The comment now says so, and that recovery is a
fault handler's job rather than the scheduler's.

eos_task_suspend() refuses a slot whose entry is NULL, which is what the
doc comment now says instead of the looser "holds no task"; a slot the
canary check marked DELETED still has its entry set and is accepted.

tests/CMakeLists.txt: the reason test_linux_security_paths is POSIX-only is
that the shell-building bodies are compiled out on Windows, not that the
predicate is skipped there.
…are not releases

eos-simulation.yml and upstream-drift.yml build eBoot as a Release build of
qemu_arm64. eBoot#120's fix makes such a build refuse to configure without a
production trust anchor, because that is the shape a release has and the
development key must not ship in one. These two builds exist to run the
simulation and to see whether master still builds; nothing they produce
reaches a device, so they pass -DEBLDR_ALLOW_DEV_KEY=ON, which is the
option that gate provides for exactly this. On an eBoot pin that predates
the option the flag is an unused-variable warning from CMake, not an error,
so this is safe to land before the eBoot change.

@srpatcha srpatcha left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review — eos#152 "ci: opt the eBoot builds into the development key, as CI builds that are not releases"

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

Verdict: Correct, complete for this repository, and the "safe to land first" claim holds —
I tested all three configures against real trees rather than reasoning about them. It covers
both of the two eBoot configures eos has, and it uses the opt-out the eBoot gate was designed
around rather than working around the gate. One Low: the second call site gets the security
opt-out with no comment while the first gets a five-line one.

Findings

# Severity File:line Finding Recommended fix
1 Low .github/workflows/upstream-drift.yml:130 -DEBLDR_ALLOW_DEV_KEY=ON appears here bare, while the identical flag at eos-simulation.yml:102 carries a five-line comment at :89-93 explaining that eBoot refuses a Release build of a real board without a production anchor and that nothing this build produces reaches a device. This is a security opt-out — it is the one way past the control that stops a published eBoot artifact trusting a key whose secret is in RFC 8032 — and an unexplained instance of it is exactly the line someone copies into a workflow where it does not belong. eBoot's own ci.yml:104-106 comments its use of the same flag for the same reason, so the commented form is the established pattern in both repos; this is the odd one out. Two lines above :127: # EBLDR_ALLOW_DEV_KEY: this job builds eBoot master to see whether it still builds; nothing it produces reaches a device. Never use this flag in a workflow that publishes an artifact. The second sentence is the part that travels with a copy-paste.

One finding, Low, on an 11-line workflow change. There is nothing else wrong with it.

Verification performed for this review

Detached worktrees under .ai/autoreview/state/scratch/. The eos checkout was clean before
and is clean now; nothing was committed or pushed.

The three claims that matter are about how this flag behaves against three different eBoot
trees, so I configured all three rather than reading about them. Host compiler, no cross
toolchain — the gate and the unused-variable behaviour are both toolchain-independent, which is
what these tests are about.

Check Result
A — "safe to land first". Pinned eBoot a172a6d (predates the option) + -DEBLDR_ALLOW_DEV_KEY=ON, with this PR's other flags exit 0, and CMake emits CMake Warning: Manually-specified variables were not used by the project. A warning, not an error, exactly as the body claims. This PR can merge before eBoot#122 without breaking anything. Verified.
B — is the premise real? eBoot#122's tree + -DEBLDR_BOARD=qemu_arm64 -DCMAKE_BUILD_TYPE=Release, without the flag exit 1 — EBLDR_PRODUCTION_KEY is not set: a Release build of board 'qemu_arm64' …. The breakage this PR prevents is real and names the exact board these workflows build, not a hypothetical.
C — does the fix work? Same tree, with -DEBLDR_ALLOW_DEV_KEY=ON exit 0, -- Trust anchor: RFC 8032 test key (development only; see core/keystore.c).
Are these the only eBoot configures in eos? YES — git grep EBLDR_BOARD across .github/workflows/*.yml at this head returns exactly two hits, eos-simulation.yml:99 and upstream-drift.yml:127, and both now carry the flag. cross-repo-dispatch.yml mentions eboot but only POSTs a dependency-update event; it builds nothing. The change is complete for this repository.
pytest tests/unit -q PASS — 43 passed, 0.37s
eos#150 exists and matches CONFIRMED — open: "CI builds of eBoot must opt into the development key once eBoot#120's gate lands"
EBOOT_COMMIT pin CONFIRMED — eos-simulation.yml:19 pins a172a6d6e1e66877413ed546401a65ee4f4f90db, the revision tested in A.

Was anything weakened? This is the one question worth asking carefully, because the diff
literally adds a flag whose name is "allow the development key". The answer is no, for a
reason specific to what these two jobs are: neither publishes anything. eos-simulation.yml
builds eBoot to run the QEMU simulation; upstream-drift.yml builds eBoot master nightly to
answer "does it still compile" — its own failure message at :147 says so ("eBoot master does
not build. eos PRs are unaffected (they build the pin)"). Nothing either job produces is
collected, uploaded or released. That is precisely the case eBoot#122 created the option for,
and taking the documented opt-out is the right move; the wrong move would have been passing a
real production key into a CI build, or pressuring eBoot to relax the gate.

Note the asymmetry between the two jobs, because it is the reason this PR is correctly
scoped.
eos-simulation.yml builds the pin, so the flag is inert there until
EBOOT_COMMIT is bumped past eBoot#122 — belt and braces, harmless now, necessary later.
upstream-drift.yml builds eBoot master, so the flag becomes load-bearing the night
eBoot#122 merges. Both needed it; only one needs it today.

Interaction with the eBoot#122 review

Recorded because these two PRs have to be read together. My review of eBoot#122 in this same
run raises two High findings against that gate: it matches only the literal string Release
(so MinSizeRel, RelWithDebInfo, lowercase release and an unset build type all configure a
real board with the published dev key), and it never checks that a supplied production key is a
point on the curve. Neither weakens this PR. These workflows pass
-DCMAKE_BUILD_TYPE=Release, the spelling the gate does catch, so they need the opt-out with or
without those fixes; and if eBoot#122's gate is later widened to cover the other release-shaped
build types — which is what I recommended there — these two lines keep working unchanged,
because an explicit opt-out is checked before any build-type test. This PR is stable against
that fix.

Architecture conformance

Conforms. §21: eos is Tier 1 — Foundation, and .github/workflows/ is Infrastructure under
the same section; both files are in the owning repo. §5.1's dependency direction is untouched —
a CI job checking out a sibling repository to build it is not a runtime dependency, and eos's
library and kernel gain no edge to eBoot from this change. §14.1 ("Integrate key management
across eBoot, eSec, eOTA and release signing") is the clause this sits under, and the change is
what integration looks like in practice: a policy eBoot enforces at its own configure boundary,
honoured explicitly by a consumer instead of routed around. §28's status policy is served by the
comment at eos-simulation.yml:89-93, which states what the build is and is not — finding 1 is
only that the second site does not say the same thing.

No architecture proposal appended. The §14.1 proposal filed during the eBoot#116 review —
that the master design should require a release build to carry a production anchor — already
covers this ground, and this PR is a consumer honouring that rule rather than evidence the rule
is wrong. Nothing here shows the master design stale or silent.

Proposed changes

Optional:
  upstream-drift.yml:127  add the rationale comment (finding 1)

Merge order — this one has a deadline attached:
  #144 first (this is stacked on it).
  Land this BEFORE eBoot#122, or the nightly `Upstream drift` job goes red at
  configure the next time it runs. Verified safe to land first: on the current
  pin the flag is a warning, exit 0.
  Bump EBOOT_COMMIT only after eBoot#115 lands.

Not checked

  • Neither workflow was executed. I verified the CMake behaviour the change depends on, on
    three real trees, but I did not run eos-simulation.yml or upstream-drift.yml. That the
    jobs still pass end to end with the flag is Inferred from the configure result, not
    observed.
  • The aarch64 cross toolchain path — NOT TESTED. No aarch64-linux-gnu-gcc on this host, so
    all three configures above used the host compiler and omitted
    -DCMAKE_TOOLCHAIN_FILE=toolchains/aarch64-linux-gnu.cmake. The gate and the
    unused-variable warning are evaluated by CMake before any compiler-specific work, so I do not
    expect the toolchain to change either result — but I did not confirm it, and a
    qemu_arm64 cross configure is what CI actually runs.
  • upstream-drift.yml's pin-extraction logic (:65-92, which reads EBOOT_COMMIT and
    EBUILD_COMMIT out of the other workflow) was read only far enough to establish that this job
    builds eBoot master rather than the pin. I did not audit it.
  • Whether eBoot#122 will merge in the shape I reviewed. My "stable against that fix"
    statement above assumes the explicit opt-out survives; if eBoot reworks the gate to drop
    EBLDR_ALLOW_DEV_KEY in favour of something else, this PR needs revisiting. Worth re-checking
    at merge time.
  • No eos-side test pins this. eBoot#122 added test_release_workflow_production_key.py,
    which fails if any eBoot release configure opts into the dev key. eos has no equivalent guard,
    so nothing would catch -DEBLDR_ALLOW_DEV_KEY=ON being added to a future eos workflow that
    does publish something. Today eos publishes no eBoot artifact, so there is nothing to guard —
    recorded as a gap in coverage, not a finding against this PR.

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

Review follow-up on embeddedos-org#152. -DEBLDR_ALLOW_DEV_KEY=ON appeared bare in
upstream-drift.yml while the identical flag in eos-simulation.yml carries a
comment explaining it, as does eBoot's own ci.yml. The flag is the one way
past eBoot's refusal to build a Release image of a real board on the RFC 8032
test key, so an unexplained copy of it is exactly the line that gets pasted
into a workflow that publishes an artifact. The comment now travels with it,
and the second sentence is the part meant to survive a copy-paste.

Comment only. Verified the parsed workflow is structurally identical before
and after (same 8 steps, same run-block commands, flag still present), the
run block passes bash -n, and pytest tests/unit is 43 passed.
@Kartikey1306

Copy link
Copy Markdown
Contributor Author

Finding 1 addressed at a44179d: the rationale comment now sits directly above the cmake call in upstream-drift.yml, in the same voice as eos-simulation.yml:89-93 and eBoot's ci.yml:104-106, with the copy-paste guard as its own sentence ("Never use this flag in a workflow that publishes an artifact").

Comment-only change, and verified as such at a44179d in this shell: the parsed workflow is structurally identical before and after (same 8 steps, same 8 non-comment run-block commands, -DEBLDR_ALLOW_DEV_KEY=ON still present), the run block passes bash -n, pytest tests/unit -q → 43 passed. The deadline note in your proposed changes stands as written — land this before eBoot#122, bump EBOOT_COMMIT only after eBoot#115.

Merge order unchanged: #144 first. Review b55c147..a44179d for this round.

@srpatcha srpatcha left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review — eos#152 "ci: opt the eBoot builds into the development key, as CI builds that are not releases"

head: a44179d author: Kartikey1306 ci: pass (29 green, 0 red; Create GitHub Release skipped)

Verdict: Follow-up review of b55c1470..a44179d7 (one commit, four comment lines).
The previous finding is resolved, and I confirmed the change is comment-only by parsing the
workflow and comparing it with comment lines stripped — identical. One new Medium, and it is new
information rather than something the last review missed: eBoot#122 has since widened its gate
to refuse every non-Debug build type including an unset one, and that pulls eight documented
cmake commands in this repository into scope. This PR fixed eos's CI against that gate and left
eos's own getting-started instructions pointing at a command that will stop working.

Previous finding — status

# Previous finding Status Evidence
1 Low — -DEBLDR_ALLOW_DEV_KEY=ON appears bare at upstream-drift.yml:130 while the identical flag in eos-simulation.yml carries a five-line rationale; an unexplained security opt-out is the line someone copies where it does not belong Resolved in a44179d .github/workflows/upstream-drift.yml:125-128. The comment sits directly above the cmake call, in the same voice as eos-simulation.yml:89-93 and eBoot's ci.yml:104-106, and it keeps the copy-paste guard as its own sentence — "Never use this flag in a workflow that publishes an artifact" — which is the part that travels. It also names what the flag opts out of, which the recommended wording did not.

Findings

# Severity File:line Finding Recommended fix
1 Medium (P2) GETTING_STARTED.md:249; docs/integration-guide.md:85,194; docs/adding-hardware.md:134; docs/three-way-alignment.md:209; DEPLOYMENT.md:133,182; docs/book/part4-ecosystem/ch17-eboot.md:92 Eight documented eBoot cmake commands in this repository stop working the day eBoot#122 merges, and this PR is the change that was supposed to see that coming. When the last review was written, eBoot#122's gate matched the literal string Release, so only builds passing -DCMAKE_BUILD_TYPE=Release — the two CI jobs — were affected, and "are these the only eBoot configures in eos? YES" was the right answer to the right question. eBoot#122 has since moved at 302fc5f to exempt only Debug and refuse everything else including an unset CMAKE_BUILD_TYPE, which I verified against that PR's head. That changes the blast radius, and I measured it both ways rather than reasoning about it: cmake -B build -DEBLDR_BOARD=nrf52 — the command at GETTING_STARTED.md:249, verbatim, nothing else on the line — exits 0 on eBoot origin/master (221325c) and exits 1 on eBoot#122's head (8d09efb) with "EBLDR_PRODUCTION_KEY is not set: a build with no CMAKE_BUILD_TYPE of board 'nrf52' would compile in the RFC 8032 test key…". Same for stm32f4. All eight sites pass a real board and set neither a build type nor the opt-out. (DEPLOYMENT.md:177 uses EBLDR_BOARD=none and is correctly exempt.) The failure is loud and the message says what to pass, so nobody is silently endangered — but the first command in this repository's getting-started guide failing is a §25.2/§39 problem, and it is this PR's subject matter: CI and the docs break for one reason, and only CI was fixed. Same treatment the workflows got, with the build type rather than the opt-out, because a reader following GETTING_STARTED.md is doing a development build: cmake -B build -DEBLDR_BOARD=nrf52 -DCMAKE_BUILD_TYPE=Debug. Verified working against eBoot#122's head. Where the snippet is explicitly about producing a device image (DEPLOYMENT.md:133), say what a real deployment needs instead: -DEBLDR_PRODUCTION_KEY=<64 hex characters>. Either way it is one flag per site, and it can land before eBoot#122 without breaking anything on today's master — -DCMAKE_BUILD_TYPE=Debug is valid on both. If you would rather keep this PR to the two workflow lines, that is a fair call; then it needs its own issue with the eBoot#122 deadline attached, because otherwise it is discovered by whoever follows the guide next.

Verification performed for this review

Detached worktrees under .ai/autoreview/state/scratch/ for this head, eBoot#122's head and eBoot
origin/master. The eos checkout was clean before and after and was not touched; nothing was
committed or pushed.

Check Result
Is a44179d comment-only? CONFIRMED. Parsed both revisions of upstream-drift.yml with PyYAML, stripped # lines from every run: block, and compared: identical. 8 steps, 5 run steps, EBLDR_ALLOW_DEV_KEY=ON still present. Raw-YAML comparison differs only because the comment lives inside a block scalar.
bash -n over every run: block PASS
pytest tests/unit -q PASS — 43 passed, 0.36s
Is the opt-out still honoured by eBoot#122's final gate? Ran -DEBLDR_BOARD=stm32f4 -DCMAKE_BUILD_TYPE=Release -DEBLDR_ALLOW_DEV_KEY=ON against 8d09efb rc 0, Trust anchor: RFC 8032 test key. The last review called this "stable against that fix" and marked it Inferred; it is now Verified — NOT EBLDR_ALLOW_DEV_KEY survived the rewrite and is still evaluated before the build-type test.
GETTING_STARTED.md:249 verbatim against eBoot master 221325c rc 0 — works today
Same command against eBoot#122 head 8d09efb rc 1, "a build with no CMAKE_BUILD_TYPE of board 'nrf52'" — finding 1
Same pair for stm32f4 rc 0 / rc 1 — same result
Every EBLDR_BOARD= in the repository at this head 10 sites: 2 in .github/workflows/ (both now carry the flag and a comment) and 8 in documentation (none carries a build type or the opt-out), plus DEPLOYMENT.md:177 with EBLDR_BOARD=none, which the gate exempts. The workflow half of the change is complete; the documentation half does not exist.
Was anything weakened? No. Four comment lines. The security posture is unchanged, and the comment strengthens it — an opt-out that explains itself is harder to copy into a publishing workflow than a bare flag.

Interaction with the other PRs in this run

Recorded because these have to be read together, and because it is where finding 1 came from.

  • eBoot#122 (reviewed in this run at 8d09efb): its gate now refuses Release,
    RelWithDebInfo, MinSizeRel, any casing, and an unset build type, exempting only Debug.
    That is the correct fix and I recommended it — it is also what turns finding 1 from nothing into
    a Medium. The two lines this PR adds keep working unchanged.
  • eBoot#129: both eos jobs pass -DEBLDR_VERIFY_STAGE1=OFF, which is correct and worth
    saying plainly rather than flagging — qemu_arm64 has no stage-1 linker script, so
    EBLDR_VERIFY_STAGE1=ON would hit eBoot's FATAL_ERROR at CMakeLists.txt:354. It does mean
    neither eos job exercises the stage-1 measurement path, which is fine for what they are for, and
    is the reason the P0 I raised on eBoot#129 is not visible from this repository.

Architecture conformance

Conforms; unchanged from the previous review. §21: eos is Tier 1 — Foundation and
.github/workflows/ is Infrastructure under the same section; both files are in the owning repo.
§5.1 is untouched — a CI job checking out a sibling repository to build it creates no runtime
dependency, and no eos target gains an edge to eBoot. §14.1 ("integrate key management across
eBoot, eSec, eOTA and release signing") is the clause this sits under, and the change is what
integration looks like in practice: a policy eBoot enforces at its configure boundary, honoured
explicitly by a consumer rather than routed around. §28's status policy is served by both comments
now stating what the build is and is not. Finding 1 is a §25.2/§29 gap — "fast onboarding" and
"Getting Started — simulator and first supported board" — not a §5.1 one.

No architecture proposal appended. The §14.1 proposal filed during the eBoot#116 review already
covers this ground, and this PR is a consumer honouring that rule rather than evidence the rule is
wrong.

Proposed changes

Optional in this PR, otherwise its own issue with the eBoot#122 deadline:
  GETTING_STARTED.md:249            -DCMAKE_BUILD_TYPE=Debug
  docs/integration-guide.md:85,194  -DCMAKE_BUILD_TYPE=Debug
  docs/adding-hardware.md:134       -DCMAKE_BUILD_TYPE=Debug
  docs/three-way-alignment.md:209   -DCMAKE_BUILD_TYPE=Debug
  docs/book/.../ch17-eboot.md:92    -DCMAKE_BUILD_TYPE=Debug
  DEPLOYMENT.md:133,182             -DEBLDR_PRODUCTION_KEY=<64 hex> for the
                                    deployment snippet; Debug for the dev one
  All eight are valid on today's eBoot master too, so they can land first.

Merge order — this one still has a deadline attached:
  #144 first (this is stacked on it).
  Land this BEFORE eBoot#122, or the nightly `Upstream drift` job goes red at
  configure. Re-verified safe to land first.
  Bump EBOOT_COMMIT only after eBoot#115 lands.

No fix PR opened. Finding 1 is Medium and the brief's autofix rule is High-only. It is also a
change I could verify only against a branch that has not merged: adding -DCMAKE_BUILD_TYPE=Debug
is harmless on today's master and necessary after eBoot#122, but "necessary after" is a prediction
about a PR that is still open, and an unattended agent should not commit documentation on that
basis.

Not checked

  • Neither workflow was executed. I verified the CMake behaviour they depend on against three
    real eBoot trees; I did not run eos-simulation.yml or upstream-drift.yml. End-to-end pass is
    Inferred from the configure results.
  • The aarch64 cross toolchain path — NOT TESTED. No aarch64-linux-gnu-gcc on this host, so
    every configure above used the host compiler and omitted
    -DCMAKE_TOOLCHAIN_FILE=toolchains/aarch64-linux-gnu.cmake. The gate is evaluated before any
    compiler-specific work, so I do not expect the toolchain to change the result — but a
    qemu_arm64 cross configure is what CI actually runs, and I did not reproduce it. Unchanged
    from the previous review.
  • The eight documented commands were tested for the configure step only, and only for
    nrf52 and stm32f4. I did not check whether my_board (docs/adding-hardware.md:134, a
    placeholder) or the subsequent cmake --build steps work; the finding is about the gate, which
    fires at configure.
  • upstream-drift.yml's pin-extraction logic (:65-92) was not audited, only read far enough
    to confirm this job builds eBoot master rather than the pin.
  • Whether eBoot#122 merges in the shape I reviewed. Finding 1 and the "stable against that fix"
    result both describe 8d09efb. If the gate changes again, re-check.
  • No eos-side test pins any of this. eBoot#122 has
    test_release_workflow_production_key.py, which fails if an eBoot release configure opts into
    the dev key; eos has no equivalent guard, so nothing would catch -DEBLDR_ALLOW_DEV_KEY=ON
    appearing in a future eos workflow that does publish something, and nothing would catch a ninth
    undocumented-build-type cmake line being added to the docs. Today eos publishes no eBoot
    artifact, so the first half is a gap with nothing behind it yet.
  • mergeStateStatus is BLOCKED — REVIEW_REQUIRED plus the #144 stack.

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

…he key a device needs

Review follow-up on embeddedos-org#152. This PR opted the two CI builds of eBoot into
the development key ahead of eBoot#122's gate; the review measured that
eBoot#122 has since moved to exempt only Debug and refuse an unset
CMAKE_BUILD_TYPE, so the eight documented `cmake -B ... -DEBLDR_BOARD=<real
board>` commands in this repository -- none of which set a build type --
stop working the day it merges. CI and the docs break for one reason; only
CI had been fixed.

Seven development snippets (GETTING_STARTED.md:249, integration-guide.md:85
and :194, adding-hardware.md:134, three-way-alignment.md:209,
DEPLOYMENT.md:182, ch17-eboot.md:92) now pass -DCMAKE_BUILD_TYPE=Debug: a
reader following the guide is doing a development build, and Debug is what
the gate exempts. The one deployment snippet (DEPLOYMENT.md:133, "flash
eBoot first") says what a device image actually needs instead --
-DEBLDR_PRODUCTION_KEY=<64 hex characters: your Ed25519 public key> -- since
a Debug build is not what goes on a device. DEPLOYMENT.md:177 uses
EBLDR_BOARD=none and is exempt as it was.

Measured, not read, against both trees with configure-only runs:
  GETTING_STARTED.md:249 verbatim   eBoot master 682d005 rc=0   eBoot#122 1cd1265 rc=1
  ... + -DCMAKE_BUILD_TYPE=Debug     master rc=0                embeddedos-org#122 rc=0
  DEPLOYMENT.md:133 + a real Ed25519 public key (openssl genpkey)
                                    master rc=0                embeddedos-org#122 rc=0,
                                    "Trust anchor: production key from EBLDR_PRODUCTION_KEY"
so every edit is valid on today's master and this can land before eBoot#122.
A 64-character dummy is refused by cmake/ProductionKey.cmake ("not a usable
Ed25519 public key"), which is why the placeholder says what the key is.

The review's own sweep, re-run: no `cmake -B ... -DEBLDR_BOARD=<board>` in
any .md is left without a build type or a key within its command.
pytest tests/unit 43 passed. three-way-alignment.md:209 is the same line in
embeddedos-org#151's rewrite of that file, so whichever of embeddedos-org#151/embeddedos-org#152 lands second rebases
without conflict.
@Kartikey1306

Copy link
Copy Markdown
Contributor Author

Taken in this PR at cd95cf7, since it is the same defect as the CI half: eight documented eBoot configures, none with a build type, all fixed.

Seven development snippets — GETTING_STARTED.md:249, integration-guide.md:85 and :194, adding-hardware.md:134, three-way-alignment.md:209, DEPLOYMENT.md:182, ch17-eboot.md:92 — now pass -DCMAKE_BUILD_TYPE=Debug. The deployment snippet at DEPLOYMENT.md:133 says what a device image needs instead: -DEBLDR_PRODUCTION_KEY=<64 hex characters: your Ed25519 public key>. DEPLOYMENT.md:177 (EBLDR_BOARD=none) untouched, as you noted.

Measured in this shell, configure-only, rather than taking the finding on trust — and it reproduced exactly:

command eBoot master 682d005 eBoot#122 1cd1265
cmake -B build -DEBLDR_BOARD=nrf52 (:249 verbatim) rc=0 rc=1 — EBLDR_PRODUCTION_KEY is not set: a build with no CMAKE_BUILD_TYPE …
… -DCMAKE_BUILD_TYPE=Debug rc=0 rc=0
-DEBLDR_BOARD=stm32f4 -DEBLDR_PRODUCTION_KEY=<real key> (openssl-generated) rc=0 rc=0, Trust anchor: production key from EBLDR_PRODUCTION_KEY
same with a 64-char dummy rc=0 rc=1 — ProductionKey.cmake:62, not a usable Ed25519 public key

So every edit is valid on today's master and this still lands before eBoot#122. The last row is why the placeholder names what the key is. Your sweep re-run at cd95cf7: no cmake -B … -DEBLDR_BOARD=<board> in any .md is left without a build type or a key in its command. pytest tests/unit -q → 43 passed. Body updated with the widened scope.

One honest note: three-way-alignment.md on this branch is the pre-#151 version (the two PRs are siblings on #144); :209 is the same line in #151's rewrite, so they rebase cleanly in either order. Range for this round: a44179d..cd95cf7, six files, +9/−6. Merge order unchanged — #144 first, this before eBoot#122.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CI builds of eBoot must opt into the development key once eBoot#120's gate lands

2 participants