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

[Kartikey1306]

services/linux/src/linux_security.c builds shell commands with snprintf and hands them to system() or popen() — nine call sites. is_path_safe() is the only thing standing between a caller's input and the shell.

The defect

It rejected ;|&><$()"'. It did not reject the backtick — command substitution in every POSIX shell — or a newline, which ends one command and starts the next. It was applied at two of the nine call sites. And is_path_safe(NULL) returned 1. Compiled verbatim from master:

  /tmp/`touch /tmp/pwned`      -> SAFE    (backtick command substitution)
  /tmp/x\ntouch /tmp/pwned     -> SAFE    (newline starts a second command)
  (NULL)                       -> SAFE
  /tmp/a;id                    -> reject

Against the unfixed file the test suite fails and the injected command runs:

uid=501(kartikey) gid=20(staff) groups=20(staff),12(everyone),61(localaccounts),...
[FAIL] eos_busybox_build(&bb) != 0
[FAIL] eos_ima_sign_file(&ima, "/tmp/`id`") != 0

That uid= line is id executing out of a path string.

A fourth hole, found in review: eos_busybox_configure() validated source_dir before filling in its default, and is_path_safe("") returns 1 because its loop never runs. So the default path — the one a caller gets by not setting the field — validated the empty string and handed .eos/build/src/busybox-<version> to system() unchecked, with version validated nowhere. The fill now happens before the check, and eos_busybox_set_version() refuses at the boundary as well.

The fix

• is_path_safe(): rejects backtick, newline, other control characters and backslash; NULL is unsafe; applied at every entry point that builds a command.
• is_word_safe() — for defconfig and cross_compile, which are interpolated unquoted — is now an allowlist: isalnum() plus ._/+=:-, and no leading -. It was a second denylist (space, tab, *, ?, ~) that missed [/] and {/}; answering an incomplete denylist with another denylist only moves the next gap further out.
• Security steps no longer report success without having run. Five functions ended their commands with || true or || echo … and discarded system()'s result, so a missing evmctl or setfiles, a failed policy or key copy, and a make install that never ran all returned 0. Each now separates "the step completed" from "the tool is absent or failed", logs through EOS_ERROR, and returns -1.
• offset += snprintf(...) in both busybox functions accumulated the would-be length; a truncating segment would have put the write pointer past the end of cmd and underflowed the remaining size. Unreachable at the current field widths (512 + 128 + 128 against 2048), and now checked rather than inferred.
• eos_core is a PRIVATE dependency of eos_linux_security. It was PUBLIC to let one .c file see eos/log.h; nothing under include/ needs it.
• Test fixtures are created 0600 via open(O_CREAT|O_EXCL) rather than fopen("w"), which creates 0666-masked-by-umask (CodeQL, three sites).

Contract change — read this one

eos_ima_sign_file(), eos_selinux_install_to_rootfs(), eos_selinux_label_rootfs(), eos_ima_install_to_rootfs() and eos_busybox_install_to_rootfs() previously always returned 0. They can now return -1. Also: eos_selinux_label_rootfs() with SELinux enabled and no file_contexts returns -1 where it used to echo a "skipping" line and return 0; and the guard in eos_ima_install_to_rootfs() sits above the mode == EOS_IMA_OFF early return, so a malformed rootfs_dir is refused even with IMA off.

Caller survey, since that is the question this change turns on: outside its own header and the test file, nothing in this tree calls any of these five functions — nothing in systems/, cmd/ or examples/. No caller can break on the new -1. It is in its own commit (087142b).

Validation

tests/test_linux_security_paths.c — 17 sub-tests. 11 shell payloads per path entry point, 12 strings that are not one shell word plus 16 real values that must still be accepted, one hostile rootfs_dir per *_install_to_rootfs, and the absent-tool / failed-copy case for each security step. Refusal is asserted by side effect — a sentinel file a successful injection would create — because these functions have several ways to return -1 and only one of them means the guard caught it.

Every fix is pinned by a test that fails without it. Eleven reversions, each caught:

mutation	result
|| true restored on the SELinux policy copy	1 check failed
|| echo restored on setfiles	1
the no-file_contexts branch back to return 0	1
|| true restored on the IMA key copy	1
make install's result discarded again	2
an unreadable policy_file swallowed again	1
is_word_safe() back to the old denylist	1
rootfs_dir dropped from the SELinux guard	1 — and the injected touch ran
rootfs_dir dropped from the busybox guard	1 — and the injected touch ran
rootfs_dir dropped from the IMA guard	1
/init's fopen() silent again	1

Two things that came out of doing this rather than asserting it:

• The first SELinux hostile-rootfs_dir case did not discriminate. A hostile rootfs_dir fails at the fopen() long before any command is built, so it passed on an unguarded build too. It now uses a real directory whose name is the injection, with the /etc the function needs and a loaded policy, so the cp is genuinely reached.
• The counter-checks caught a fixture bug that had two other tests passing for the wrong reason: these functions MKDIR("<rootfs>/etc/<x>") without creating <rootfs>/etc first, so a bare mkdtemp() rootfs fails at the fopen(), not at the step under test.

cmake --build … && ctest --no-tests=error   ->  40/40 passed
./tests/test_linux_security_paths           ->  17/17 sub-tests PASS
pytest tests/ -q                            ->  15 passed

40, not the 35 recorded in an earlier revision of this description — the branch has been rebased onto a newer master since.

Not covered

• No host has veritysetup, setfiles or evmctl installed, here or in CI. The dm-verity and SELinux assertions that rely on a return code do not discriminate under that condition; the sentinel-based ones do, anywhere. That is why the coverage added in this round is side-effect based.
• eos_kaudit_install_to_rootfs() and the /init writer interpolate rootfs_dir into snprintf paths and fopen them. No shell is involved, so is_path_safe is not the right control there — path traversal through rootfs_dir is a different question and this PR does not address it.
• Windows was not built. The #else arms now return -1 where several returned 0, for steps that cannot run there. No test covers it.
• Neither predicate has a fuzz harness in tests/fuzz/. Pre-existing gap; not closed here.
• The -Wformat-truncation= warning reported on the GCC leg is fixed by sizing the buffers from each other and asserting snprintf's return, but there is no GCC on this machine, so that one is verified by construction and by CI rather than reproduced locally.

Merge order

tests/CMakeLists.txt carries the test_crypto_ed25519_loworder registration block, because origin/master has that test file and never registers it, which fails tests/unit/test_cmake_test_registration.py. #118 and #127 add the identical block at the identical anchor, and #126 adds a stray blank line at the same anchor — whichever lands second conflicts. Land #127 first, then rebase this one and drop the duplicated hunk. (An earlier note here named #125; it is closed, and #127 is its replacement.)

Left for a separate change

These commands would be better run through fork/execv with an argv array than validated as strings — validation is a weaker guarantee than never involving a shell. Two of them are pipelines (veritysetup format … | grep | awk) and would need restructuring, so that is a larger change than this one and it is not smuggled in here.

[srpatcha]

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

head: a064481 author: Kartikey1306 ci: pass

Verdict: The three diagnosed holes are real and the fixes for them are correct, and
the counter-check that a well-formed path still reaches the shell is the right instinct.
But the guard on eos_busybox_configure() is placed above the line that populates the
field it protects, so the default path — the one taken whenever a caller does not set
source_dir — still reaches system() with unvalidated data; and the file's larger
fail-open problem, security steps whose exit status is discarded, is untouched while the
PR's Validation section reads as though the file is now sound.

Findings

#	Severity	File:line	Finding	Recommended fix
1	High	services/linux/src/linux_security.c:481-484 (post-patch: guard inserted above :482)	The new guard is bypassed on the default path. The check runs first, then if (!bb->source_dir[0]) builds source_dir from bb->version — and bb->version is validated nowhere: eos_busybox_set_version() (:442-445) is a bare strncpy. After eos_busybox_init() zeroes the struct, source_dir is empty, is_path_safe("") returns 1 because the loop body never executes, the guard passes, and .eos/build/src/busybox-<version> goes into make -C "%s" at :488. A backtick in version is command substitution inside those double quotes. This is the same injection class the PR closes elsewhere, on the path a caller gets by default, in a function the PR modifies. It also persists: configure writes the poisoned string back into bb->source_dir, so eos_busybox_build()'s guard will reject it afterwards — after configure already ran the command.	Validate in eos_busybox_set_version() (return -1 on !is_word_safe(version)), and move the configure guard below the default-fill at :485 so it validates the value actually used. Both, not either: the setter stops the bad value entering, the re-ordered guard stops any other route to the same field. Add the case to test_busybox_refuses_hostile_fields() — empty source_dir plus version = "1.36.1\touch /tmp/pwned`"— driven throughtest_injection_does_not_execute()`'s sentinel so it discriminates.
2	High	services/linux/src/linux_security.c:191,195 (also :82, :104, :177, :527)	Security steps report success without having run. eos_ima_sign_file() discards system()'s return entirely and ends return 0;, and the command it built ends || echo 'evmctl not available', so the shell's own exit status is masked too. On any host without evmctl the function signs nothing and tells its caller it signed. .ai/security.md, "Fail closed": "A verification step that cannot run must fail, not pass" and "A security step whose result is discarded is a finding even when the happy path is correct." The same discard is at eos_selinux_install_to_rootfs():82, eos_selinux_label_rootfs():104 (|| echo '…skipping labeling'), eos_ima_install_to_rootfs():177 (|| true) and eos_busybox_install_to_rootfs():527. Pre-existing, not introduced here — raised against this PR because it modifies four of those five functions and its Validation section presents the file as hardened.	Separate "the tool is absent" from "the tool ran and failed", and never collapse either into 0. Drop the || echo … / || true tails, keep rc = system(cmd), and return non-zero when the step did not complete. If a missing evmctl/setfiles must be tolerated, make that an explicit caller-visible state (a distinct return code or a flag on the struct) rather than a 0 indistinguishable from success. eos_dmverity_verify():309-311 already does this correctly and is the pattern to copy.
3	Medium	tests/test_linux_security_paths.c:356-363	The suite covers eos_dmverity_verify, eos_dmverity_create, eos_busybox_build, eos_busybox_configure and eos_ima_sign_file. It covers none of eos_selinux_install_to_rootfs, eos_selinux_label_rootfs, eos_ima_install_to_rootfs or eos_busybox_install_to_rootfs — including the specific gap the PR body names as its second finding, rootfs_dir reaching cp unchecked through path at :80. That guard (services/linux/src/linux_security.c:70-76) ships with no test.	Add one hostile-rootfs_dir case per *_install_to_rootfs entry point. eos_selinux_install_to_rootfs() and eos_ima_install_to_rootfs() create directories under rootfs_dir, so give them a mkdtemp() root the way test_ordinary_paths_still_reach_the_shell() does and assert both refusal and the absence of the sentinel.
4	Medium	services/linux/src/linux_security.c:61-68 (is_word_safe)	The PR's own argument is that a denylist for shell metacharacters is the wrong shape — it was incomplete, and the two missing entries were exploitable. is_word_safe() answers that with a second denylist: space, tab, *, ?, ~. It does not reject [ or ] (glob character class) or { / } (brace expansion, which /bin/sh performs when it is bash). Both reach make unquoted through defconfig at :489 and cross_compile at :492. The consequence is argument mangling rather than command execution, which is why this is Medium — but the list will need extending again, which is the failure mode the PR is fixing.	"One shell word" has a small positive definition; write it as one. Replace the character loop with an allowlist — isalnum() plus ._/+=:- — and reject everything else. That covers every real defconfig, cross_compile prefix and hashalgo value, cannot be incomplete, and needs no future maintenance.
5	Low	services/linux/src/linux_security.c:139-141	The new guard is inserted above if (ima->mode == EOS_IMA_OFF) return 0;, so eos_ima_install_to_rootfs() with IMA disabled and a malformed rootfs_dir now returns -1 where it previously returned 0. Failing closed is the right call, but it is a change to the function's contract that the body does not mention and no test pins.	Keep the ordering; say so in the body, and add a case asserting -1 for EOS_IMA_OFF with a hostile rootfs_dir so the choice is deliberate and recorded.
6	Low	net/src/net_posix.c:16,216; tests/test_net.c:213-250	Two files outside the stated scope change with no mention in the PR body, which describes only linux_security.c and the new test. Both changes are correct and worth having — the #if EOS_ENABLE_NET wrapper restores what 8480276 (#103) established and 3df89a8 (#89) undid by recreating the file without it (master has neither guard), and the ten assert → CHECK conversions finish what 39640bb (#104) started: master still has bare assert() at tests/test_net.c:213-250, and under the Release/NDEBUG build those vanish together with the calls inside them, which is precisely the hang the comment at tests/test_net.c:13-17 warns about. Being right does not make them in scope for a shell-injection PR.	Split them into their own PR, or state them in the body. Separately, that comment names the wrong macro twice — it reads "Not CHECK():" and "NDEBUG deletes CHECK()" where both mean assert(); this PR puts ten more converted lines under it, so it is worth one word of repair while you are there.
7	Low	tests/test_linux_security_paths.c:321-354	The counter-check invokes make on the test host. Without make installed, eos_busybox_configure() returns -1, CHECK(... == 0) fails, and the suite reports a failure that says nothing about the guard.	Probe for the tool first and print [SKIP] instead of counting a failure, the way mkdtemp failure is already handled at :328.
8	Low	tests/test_linux_security_paths.c:210-221,268-276	Acknowledged in the body and re-stated here because it is a real limit on the evidence: the != 0 assertions in test_dmverity_verify_refuses_hostile_paths and ..._hostile_hash_device pass against the unfixed file on any host without veritysetup, which is every CI runner. Twelve of the suite's assertions therefore prove nothing in CI.	Mitigated, not unmitigated — test_injection_does_not_execute() (:233-266) discriminates anywhere via the sentinel, and it covers dmverity_verify, dmverity_create and ima_sign_file. Extending that sentinel technique to the remaining entry points (see finding 3) would let the non-discriminating assertions be deleted rather than explained.

No check is weakened by this PR: the ten assert → CHECK conversions in tests/test_net.c
strengthen them (CHECK at tests/test_net.c:18-23 calls exit(1) and, unlike assert,
survives NDEBUG), no test is disabled, no lint loosened, no permission widened. All 26
required checks in checks.txt pass. The body's uid=501(kartikey) transcript is real
evidence of the original defect, and the "Left for a separate change" section correctly
declines to smuggle in the fork/execv rewrite.

Architecture conformance

§14.1 Security principles — conforms as far as the section reaches. No primitive is
invented; evmctl, veritysetup and setfiles are reviewed external tools, which is
what §14.1 asks for. Nothing in this diff touches key material, and no key, token or
device credential appears in the added code or in the test payloads.

§5.1 architectural law — conforms. services/linux is a platform service and depends
on nothing above it; the new test links eos_linux_security only.

§4 product hierarchy — conforms. SELinux, IMA, dm-verity and kaudit sit inside eos
as the "eSec subset" §4 explicitly places in EmbeddedOS Core, so the code is in the right
repo. (These functions do build-time rootfs construction on a Linux host, which brushes
against the eBuild boundary raised in the eos#118 review, but §4's wording covers it.)

Design gap. §14.1 has five security principles and none of them governs how a security
step invokes anything: nothing in §14.1, §8.1 or §15.1 says whether a trusted component
may build a shell command string, what validating its inputs requires, or that its exit
status must be honoured. Findings 1, 2 and 4 are all the same missing rule. Proposal
appended to .ai/autoreview/proposals/2026-09.md.

Proposed changes

1. eos_busybox_set_version() (:442): if (!is_word_safe(version)) return -1;.
2. eos_busybox_configure(): move the guard added by this PR to after the default-fill
   block at :482-485, so is_path_safe(bb->source_dir) sees the constructed value.
3. Replace the is_word_safe() character loop with an allowlist (isalnum() plus
   ._/+=:-). No behaviour change for any value in examples/; removes finding 4.
4. eos_ima_sign_file(): keep rc = system(cmd), drop the || echo 'evmctl not available'
   tail, return non-zero when the signing step did not complete. Then the same for the
   four other discard sites. This one changes caller-visible behaviour and is the only
   step here that could break a caller — it deserves its own commit and its own test.
5. Tests: hostile-version case with the sentinel (finding 1); one hostile-rootfs_dir
   case per *_install_to_rootfs entry point (finding 3); tool-probe skip (finding 7).

Steps 1–3 are self-contained and safe to land together. Step 4 is a contract change and
should be separate. Step 5 should land with whichever of 1–4 it covers.

Not checked

• I did not build this branch or run ctest; the 35/35 and the uid= transcript are
  the author's. The independent evidence is checks.txt, green for this head SHA.
• Finding 1 is read from the source, not demonstrated — I did not compile a harness to
  execute the injection through bb->version. The reasoning is is_path_safe("") == 1
  (:51-55, empty string, loop body never runs) and the default-fill at :482-484
  running after the guard; both are visible in the file, but I have not run it.
• Windows: every shell-out is inside #ifndef _WIN32 and the #else arms differ per
  function (eos_busybox_configure returns -1, eos_ima_sign_file returns 0). I did not
  review the Windows behaviour of the new guards.
• I did not audit the non-shell paths in this file — eos_kaudit_install_to_rootfs()
  (:361-374) and eos_busybox_install_to_rootfs()'s /init writer (:531-540)
  interpolate rootfs_dir into snprintf paths and then fopen them. No shell is
  involved, so is_path_safe is not the right control there, but path traversal through
  rootfs_dir is a different question that this PR does not raise and I did not pursue.
• Whether any caller in the tree relies on eos_ima_sign_file() returning 0 — the
  contract change in step 4 needs that survey before it lands, and I did not do it.

---

Automated architecture review of a06448110a5a — scheduled, model claude-opus-5, checked against the EmbeddedOS Master Design v2.0. Advisory only: this reviewer never approves, requests changes, or merges. Reply here to discuss or push back — a wrong finding is a bug worth reporting.

[srpatcha]

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

head: 8dca787 author: Kartikey1306 ci: fail (both failures inherited from eBoot master — see CI)

Verdict: The injection fix is correct and the test is the real thing — I weakened the denylist by one character and watched four checks fail with the injected command actually executing, so this suite discriminates rather than rubber-stamps. What it does not do is finish the job it starts: the PR removes one || echo + discarded-system() pair with an explicit .ai/security.md citation and leaves four more in the same file, each reporting success for a security step that did not run.

Findings

#	Severity	File:line	Finding	Recommended fix
1	High	services/linux/src/linux_security.c:110-112, :126-134, :205-207, :596-598	Four system() calls still swallow their result behind a `
2	Low	services/linux/src/linux_security.c:71-78	is_word_safe() rejects *, ? and ~ but not [ or ]. bb->defconfig is interpolated unquoted (make -C "%s" %s, :558-559), which is precisely where the shell globs, so a bracket expression still expands. Nothing rejects a leading - either, so defconfig = "--version" reaches make as an option rather than a target. Neither is command execution — glob expansion happens after tokenisation, so an expanded filename containing ; is not re-parsed — so the impact is a wrong make invocation, not injection. Flagged because a glob denylist that lists three of five metacharacters reads as an oversight rather than a decision.	Add [ and ] to the is_word_safe() set and reject a leading -. Alternatively quote the two unquoted sites and drop the second predicate entirely — that removes the weaker of the two guards, though it changes defconfig from "several make arguments" to "one argument", so it is a behaviour choice rather than a pure fix.
3	Low	services/linux/src/linux_security.c:558-566, :578-582	offset += snprintf(cmd + offset, sizeof(cmd) - (size_t)offset, …) accumulates snprintf's would-be length, not what it wrote. If any segment truncates, offset exceeds sizeof(cmd), so cmd + offset is out of bounds and sizeof(cmd) - (size_t)offset underflows to a huge size_t — an unbounded write. Not reachable today, and I checked rather than assumed: the fields cap the total at 9 + 511 + 2 + 127 = 649, then +15 + 127 = 791, then +16 = 807, against cmd[2048]. It becomes reachable the moment source_dir[512], defconfig[128] or cross_compile[128] grows, or cmd shrinks, and nothing connects the two. Pre-existing, but it is in the two functions this PR rewrites the guards of.	if (offset < 0 || (size_t)offset >= sizeof(cmd)) return -1; after each snprintf, in both functions.
4	Low	tests/test_linux_security_paths.c:226-227	New -Wformat-truncation= warning on a target built with -Wall -Wextra -Wpedantic: snprintf(hostile_version, sizeof hostile_version /* 192 */, "1.36.1\touch %s`", sentinel /* 320 */) — "output between 15 and 334 bytes into a destination of size 192". No truncation occurs in practice (sentinelis ~30 bytes frommkdtemp`), but the failure mode matters here specifically: truncation would cut the closing backtick, defanging the payload, and the test would then pass because it is no longer testing anything. A security test that can silently stop being hostile should not depend on a temp-path length.	Size hostile_version from sizeof sentinel + 32, or assert snprintf's return is < (int)sizeof hostile_version so a truncated payload fails loudly instead of passing quietly.
5	Low	services/linux/CMakeLists.txt:12	target_link_libraries(eos_linux_security PUBLIC eos_core) exports eos_core's include directories to every consumer in order to make one .c file see eos/log.h. Nothing under services/linux/include/ includes it. .ai/architect.md lists "widen a public header to make one caller compile" under "Do not", and this is the CMake form of it. Verified PRIVATE is sufficient: CMake still records a static library's PRIVATE dependency as a $<LINK_ONLY:> interface requirement, so test_linux_security_paths — which names only eos_linux_security — links and passes unchanged.	PRIVATE eos_core. Confirmed by building and running the test with it.
6	Low	tests/test_linux_security_paths.c:172-206	test_ordinary_paths_still_reach_the_shell() needs make on PATH and has no skip, unlike every other environment dependency in this file (mkdtemp failure at :180, evmctl presence at :265). Without make, eos_busybox_configure() returns -1, CHECK(… == 0) fails, and the suite reports a security failure that is really a missing build tool. test_the_default_source_dir_path_is_validated() also invokes make relative to the current working directory.	The command -v skip already used for evmctl: if (system("command -v make >/dev/null 2>&1") != 0) { printf("[SKIP] …"); return; }.
7	Low	tests/CMakeLists.txt:93-96	The test_crypto_ed25519_loworder registration block is eos#125's fix, carried here so this branch is not red on the registration guard. Necessary — origin/master has tests/test_crypto_ed25519_loworder.c but does not register it, so tests/unit/test_cmake_test_registration.py fails on master. But eos#118 and eos#126 add the identical block at the identical anchor, so #118, #119, #125 and #126 all collide in tests/CMakeLists.txt. Same situation the author flagged for #115 on #118.	Land #125 first, then rebase the other three and drop the duplicated hunk. Worth folding into the existing merge-order comment so the fourth collision is not found at merge time.

CI

Two checks red, and neither is caused by this PR — identical to eos#118:

• Cross-compile ARM64 kernel — .github/workflows/eos-simulation.yml:28-33 checks out embeddedos-org/eBoot@master and builds it. eBoot's master does not compile (eBoot/include/eos_image.h:135,142, error: 'eos_image_header_t' has no member named 'reserved'). I reproduced this by building eBoot origin/master in a clean worktree; it has three pre-existing errors from one bad merge, and eBoot#94 already targets them.
• Full-stack integration summary — needs: [build-kernel, …], so it fails purely downstream. QEMU ARM64 simulation (13 TCs) shows skipping for the same reason.

Everything else is green. Reproduced locally at head 8dca787:

cmake -S . -B bt -DEOS_BUILD_TESTS=ON && cmake --build bt   # rc=0
ctest --test-dir bt --no-tests=error                        # 40/40 passed
./bt/tests/test_linux_security_paths                        # all 9 sub-tests PASS
pytest tests/ -q                                            # 15 passed

40, not the 35 in the body — the branch has been rebased onto a newer master since that figure was recorded. Not a discrepancy worth fixing, but the body's number no longer matches the head.

Negative control, because a security test that cannot fail is worth nothing. I removed the backtick and backslash from the new denylist and rebuilt:

[FAIL] test_linux_security_paths.c:242: access(sentinel, F_OK) != 0
[FAIL] the default path executed the injection: /tmp/eos_lsp_ver_QR1JoG/pwned
4 check(s) failed

Both sentinel-based tests caught it and the injected touch genuinely ran. The suite discriminates. Worth noting what the same run shows about the rest of it: dmverity_verify refuses hostile image paths, dmverity_verify refuses a hostile hash device, dmverity_create refuses hostile paths, busybox refuses hostile fields and ima_sign_file refuses a hostile file path all reported PASS against the weakened guard. The author already says the dm-verity assertions do not discriminate without veritysetup; on this evidence the same is true of the busybox and IMA return-code assertions. The two side-effect tests are carrying the whole suite. That is not a defect — it is worth knowing which five of the nine sub-tests would notice a regression.

Architecture conformance

Conforms. §14.1 / .ai/security.md, with finding 1 as the deviation.

• §14.1, "Do not invent primitives" — not engaged; no cryptography here.
• .ai/security.md, input validation — conforms and improves. "Every externally reachable parser is attack surface … define behaviour for malformed, truncated, oversized, zero-length and duplicated input." Making is_path_safe(NULL) return 0 is the zero-length/absent case done right, and the "" case is the sharpest catch in the PR: is_path_safe("") returns 1 because the loop body never runs, and the old code validated source_dir before filling in the default, so the guard ran on the empty string and the value actually used went unchecked. Moving the fill above the check is the correct fix and the accompanying test watches for the side effect rather than the return code, which is the only thing that proves it.
• .ai/security.md, "Fail closed" — deviates, via finding 1. Four of the five instances of this file's signature defect survive the PR.
• §5.1 / tier placement — conforms. services/linux sits above core; the new eos_core dependency points downward and eos_core does not depend back, as the comment states. No product-repo dependency, nothing points up a tier. Tier 1 repo, platform-service code, correct home. Finding 5 is about PUBLIC vs PRIVATE, not direction.
• No secrets in logs — checked. EOS_ERROR at :230 logs file_path only; ima->key_file is not logged anywhere, and no key material is printed.
• Weakened checks — none. The PR adds a test and tightens four predicates; nothing was disabled or loosened.

Proposed changes

1. Finding 1, in this PR. It is the same defect class the PR is about, in the same file, and the fix is the one already written at :229 applied four more times. Leaving it means the next reader has to rediscover that eos_selinux_install_to_rootfs returning 0 means nothing.
2. Findings 4 and 6 — two lines in the test file, both about the test failing or passing for the right reason.
3. Findings 2, 3 and 5 — one-liners, no behaviour change for any valid input.
4. Finding 7 — merge order: land #125, rebase, drop the duplicated hunk.
5. The fork/execv rewrite the author defers is the right call to defer. Validation is weaker than never involving a shell, and two of these commands are pipelines; that is a genuinely larger change and it should not ride along here.

I opened no fix PR. Finding 1 is High but it is not "small and provable" in the brief's sense — changing four functions from "always 0" to "0 or -1" changes their contract for every caller, and I have not traced the callers. It belongs in this PR, whose author has the context.

Not checked

• Nothing was run against veritysetup, setfiles, evmctl or a real SELinux/IMA rootfs. None is installed here, which is the same condition as CI. Every dm-verity and SELinux assertion in this suite is therefore untested against a host where the underlying tool exists, and finding 1's consequences (a rootfs with a config naming an uncopied policy, an appraise policy with no key) were reasoned from the code, not observed on a booted system.
• The callers of these functions were not traced. Finding 1's fix changes return values, and I did not check whether anything in systems/ or cmd/ treats a non-zero return from eos_selinux_install_to_rootfs or eos_busybox_install_to_rootfs as fatal. That determines whether fixing it surfaces as a clear failure or a new crash path.
• The 11 hostile payloads in HOSTILE[] were not extended. ! (history expansion), {/} (brace expansion), #, %, and non-ASCII bytes ≥ 0x80 are all accepted by is_path_safe(); I reasoned that none is shell syntax in a non-interactive sh -c inside double quotes, but I did not test them.
• No fuzzing. .ai/security.md requires fuzz coverage for externally reachable parsers and is_path_safe/is_word_safe are the validators for nine shell commands. tests/fuzz/ exists in this repo; neither predicate has a harness there, and this PR adds none. Pre-existing gap, not assessed further.
• Windows was not built. The #else branches now return -1 where two previously returned 0 — a behaviour change on Windows that no test covers and that I did not exercise.
• Analyze (C/C++), CodeQL and Static Analysis were read as pass/fail only; their findings for this head were not reviewed.

---

Automated architecture review of 8dca78738648 — scheduled, model claude-opus-5, checked against the EmbeddedOS Master Design v2.0. Advisory only: this reviewer never approves, requests changes, or merges. Reply here to discuss or push back — a wrong finding is a bug worth reporting.

[Kartikey1306]

All seven findings addressed at 3768a14. Thank you for the negative control — the weakened-denylist run is the most useful thing anyone has posted on this PR, and it is the reason I mutation-tested every fix in this round rather than only the new ones.

Finding 1 (High) — done, in this PR, in its own commit (087142b). All four remaining sites, plus two more of the same defect I found while there: eos_ima_install_to_rootfs() also left /etc/ima/policy empty and returned 0 when a named policy_file could not be opened, and eos_busybox_install_to_rootfs() ignored the fopen() for /init, so an unwritable rootfs produced an initramfs with no /init and a 0.

You said the caller survey was the thing that had to happen first, so: outside its own header and the test file, nothing in this tree calls any of those five functions — nothing in systems/, cmd/ or examples/. grep -rn -E "eos_(selinux|ima|busybox)_(install_to_rootfs|label_rootfs|sign_file)" returns the header, the implementation and the test and nothing else. No caller can break on the new -1. The #else arms return -1 for the same reason rather than 0 for a step that cannot run there; that is a Windows behaviour change nothing covers, and it is listed under Not covered.

One correction on the citation, since a wrong quote is worth reporting: .ai/security.md has no "Fail closed" section, and the sentences "A verification step that cannot run must fail, not pass" and "A security step whose result is discarded is a finding even when the happy path is correct" are not in that file or anywhere else in the repo (grep -rn "cannot run must fail\|Fail closed" → one unrelated comment in rsa_ecc_sha512.c). The finding stands on the rule that is there — .ai/security.md, Never: "Report a check as passing without running it" — which is what the commit and the code comments cite. This is the design gap your review names at the end; the proposal in .ai/autoreview/proposals/2026-09.md looks like the right home for it.

Finding 2 (Low) — done, but as your first review's finding 4 rather than this one. I took the allowlist instead of adding [ and ]: isalnum() plus ._/+=:-, no leading -. Adding two characters to a denylist that had just been shown incomplete is the failure mode this PR is about. Pinned both ways — 12 strings that are not one shell word (including {a,b}, !, #, % and a non-ASCII byte, which you listed as untested) and 16 real values that must still be accepted, because an allowlist's failure mode is over-rejection and arm-linux-gnueabihf- has to survive it. The counter-check now passes a real CROSS_COMPILE= prefix through to a live make.

Findings 3, 5, 6, 7 — done. Truncation guards on every snprintf in both busybox functions; PRIVATE eos_core; command -v make probe with [SKIP]. On test_the_default_source_dir_path_is_validated(): it needs no make and I left it without a probe deliberately — command substitution happens while the shell expands the word, before it looks for the program, so the backtick fires whether or not make exists. There is a comment saying so.

Finding 4 — done, and the reasoning was right. Buffers sized from each other, snprintf's return asserted, and the payload's length asserted against bb.version[64], which it also has to survive intact — the same silent-defang risk one copy further along. No GCC on this machine, so that warning is fixed by construction and by CI, not reproduced locally.

Finding 7 — merge order, with one update: #125 is closed; #127 is its replacement and carries the identical block. #118 adds the identical block at the identical anchor too, and #126 adds a stray blank line at the same anchor (lines 90-93 of tests/CMakeLists.txt) — that one looks unintentional and is worth a word to its author. Land #127 first, then the rest rebase and drop the hunk. Noted in the body.

On "the two side-effect tests are carrying the whole suite" — that was the most actionable line in the review. The suite is 17 sub-tests now and the side-effect technique is extended to the entry points that had no coverage at all. Eleven mutations, each caught:

mutation	result
|| true restored on the SELinux policy copy	1 check failed
|| echo restored on setfiles	1
the no-file_contexts branch back to return 0	1
|| true restored on the IMA key copy	1
make install's result discarded again	2
an unreadable policy_file swallowed again	1
is_word_safe() back to the old denylist	1
rootfs_dir dropped from the SELinux guard	1 — and the injected touch ran
rootfs_dir dropped from the busybox guard	1 — and the injected touch ran
rootfs_dir dropped from the IMA guard	1
/init's fopen() silent again	1

Two of those cost me a rewrite, which seems worth recording:

• My first SELinux hostile-rootfs_dir case did not discriminate — a hostile rootfs_dir fails at the fopen() long before any command is built, so it passed against a build with the guard deleted. It now uses a real directory whose name is the injection, with the /etc the function needs and a loaded policy, so the cp is actually reached. That is the exact trap you have flagged before, and only the mutation showed it.
• Two other tests were passing for the wrong reason for a fixture reason: these functions MKDIR("<rootfs>/etc/<x>") without creating <rootfs>/etc, so a bare mkdtemp() rootfs fails at the fopen() rather than at the step under test. The positive control caught that, not the hostile ones.

ctest 40/40, 17/17 sub-tests, pytest 15 passed. CHANGELOG entry added — the contract change is the third bullet.

Still not covered, and stated in the body: no host here or in CI has veritysetup, setfiles or evmctl, so every return-code assertion that depends on one of those still fails to discriminate; Windows is not built; neither predicate has a fuzz harness; and path traversal through rootfs_dir into eos_kaudit_install_to_rootfs() and the /init writer is a real question that is_path_safe is the wrong control for — you raised it, I agree, and this PR does not address it.

[srpatcha]

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

head: 3768a14 author: Kartikey1306 ci: fail (2 of 26 checks — both pre-existing, see below)

Verdict: The seven findings from the previous round are genuinely closed, and I verified the fix by execution rather than by reading: clean build under GCC 15.2, ctest 40/40, and all 17 sub-tests of the new suite pass. The allowlist replacement, the fill-before-validate reordering, and the five contract changes are all correct, and the caller survey that justifies them checks out. One defect of the same class the PR exists to eliminate survives in eos_dmverity_create(), in the one function that establishes the dm-verity integrity root — and I have executed proof of it.

Findings

#	Severity	File:line	Finding	Recommended fix
1	Critical	services/linux/src/linux_security.c:355-378	eos_dmverity_create() reports success for an operation that did not run, by inferring success from a stale buffer. The function's only success test is return dv->root_hash[0] != '\0' ? 0 : -1; (:389), but dv->root_hash is never cleared before the popen. If fgets (:357) yields nothing — veritysetup absent, unreadable image, empty pipeline output — the field keeps whatever a previous call left in it, and the function returns 0. pclose's status (:362) is discarded entirely, and because the command is a pipeline ending in awk, popen's status would report awk's success rather than veritysetup's failure even if it were checked — so root_hash[0] is the only failure signal in the function, which is exactly why leaving it stale defeats the whole guard. Executed against this head (veritysetup absent, struct reused without re-init, no eos_dmverity_init between calls): rc = 0 — reported SUCCESS dv.root_hash = deadbeefdeadbeefdeadbeefdeadbeef — the previous hash dv.data_device = /tmp/definitely-not-an-image.img — a file that does not exist and eos_dmverity_generate_table() then emitted 0 0 verity 1 /tmp/definitely-not-an-image.img /tmp/h2.img 4096 4096 0 0 sha256 deadbeef… - a dm-verity table asserting a root hash for an image that was never hashed. .ai/reviewer.md is explicit: "A security step that reports success without having run is Critical." The new test suite never reaches this because every dmverity case calls eos_dmverity_init(&dv) first (tests/test_linux_security_paths.c:85,118,124,144,155,157), so the struct is always clean and the suite is green.	One line, at the top of the #ifndef _WIN32 block before the popen: dv->root_hash[0] = '\0'; Then add the regression test that the current suite structurally cannot express — reuse the struct instead of re-initialising it: eos_dmverity_init(&dv); strncpy(dv.root_hash, "deadbeef…", sizeof(dv.root_hash) - 1); CHECK(eos_dmverity_create(&dv, "/nonexistent.img", "/tmp/h.img") != 0); CHECK(dv.root_hash[0] == '\0'); While there, capture pclose's status and log through EOS_ERROR on a non-zero, so the failure is diagnosable and not only detectable.
2	Medium	services/linux/src/linux_security.c:428-429	eos_dmverity_verify() returns the raw system() status, not the file's 0/-1 contract. int rc = system(cmd); dv->verified = (rc == 0); return rc; — on a failed verification system() returns the wait status, so an exit code of 1 becomes 256, not -1. Every other function in this file, including eos_dmverity_create at :389, returns 0 or -1, and the header (services/linux/include/eos/linux_security.h:91) documents no exception. A caller written the ordinary way — if (eos_dmverity_verify(...) < 0) { /* failed */ } — reads a failed integrity verification as success, because 256 is not less than zero. dv->verified is set correctly, so the damage is limited to callers that trust the return value. The PR's own comment at :292 cites this function as the model — "Follows eos_dmverity_verify(), which already gets this right" — which is true about the fail-closed intent and not about the returned value. The new tests assert != 0 (tests/test_linux_security_paths.c:89,147) and so pass either way.	return rc == 0 ? 0 : -1; and assert == -1 rather than != 0 in the two dmverity refusal tests, so the contract is pinned.
3	Low	services/linux/src/linux_security.c:653, 672	The NULL hardening is applied inconsistently. This round correctly made is_path_safe(NULL) return 0 and added if (!bb) return -1; to eos_busybox_configure (:609) and !bb to eos_busybox_set_version (:561). But eos_busybox_build (:653) and eos_busybox_install_to_rootfs (:672) dereference bb->source_dir in their very first statement with no such guard, so a NULL bb faults before is_path_safe can refuse anything. The same asymmetry applies to se (:101, :155), ima (:219, :282) and dv (:339, :417). Given that the round's stated principle is "the wrong default for a predicate guarding command construction", the struct pointer deserves the same treatment as the string.	Add the matching if (!bb) return -1; / if (!se) … / if (!ima) … / if (!dv) … to the six functions that take a struct pointer and lack one, or state in the header that these are non-NULL preconditions. Either is fine; the current mix means a reader cannot tell which.
4	Low	services/linux/src/linux_security.c:136, 176, 266, 295	2>/dev/null is retained at every site that now depends on the exit status, so the operator learns that a security step failed but never why. The EOS_ERROR messages are a real improvement and correctly state the consequence, but they have to guess the cause — "setfiles failed or is not installed" (:179) cannot distinguish a missing binary from a malformed file_contexts from a permission denial, because the tool's own stderr was discarded. Previously the suppression was harmless because the result was ignored anyway; now that the result is load-bearing, the diagnostic is the thing that makes a -1 actionable, which is what §9.2 asks for.	Drop 2>/dev/null and let the tool's stderr reach the build log, or capture it (popen with 2>&1) and include the first line in the EOS_ERROR. The cp at :136 is the clearest case: "no such file", "permission denied" and "policy dir is empty" are three different operator actions and currently read identically.
5	Low	(CI)	2 of 26 checks fail, and neither is caused by this PR. Cross-compile ARM64 kernel fails at the Build eBoot (ARM64 qemu_arm64 board) step with eBoot/include/eos_image.h:135:23: error: 'eos_image_header_t' has no member named 'reserved' (also :142), breaking eboot_core.dir/core/bootctl.c.o and image_verify.c.o; Full-stack integration summary then fails as its dependent. This is a breakage in the eBoot repository's header, not in this diff, which touches only services/linux/ and tests/. Confirmed pre-existing: the same workflow fails on origin/master at feee2726, and eos#121, #126, #129 and #134 each report the same 2 failures. QEMU ARM64 simulation (13 TCs) is skipped as a downstream consequence. All 24 other checks pass, including Build & Test (Linux x86_64), Host Tests (x86_64), Static Analysis (cppcheck + clang-tidy), Analyze (C/C++) and CodeQL.	Nothing for this author. The eos_image_header_t.reserved static assertion needs fixing in eBoot; until then no eos PR can show a fully green run and the failure is not a signal about any of them.

Architecture conformance

Conforms. services/linux/ is a platform service — master design §21 Tier 2 — Core Platform (eSec's device/data/software concerns per §14) — and §5.1 permits platform services to depend on EoS. The change moves in the permitted direction and one notch further: services/linux/CMakeLists.txt narrows eos_core from PUBLIC to PRIVATE on eos_linux_security, so the dependency stops being re-exported to anything that links the service. That is .ai/architect.md's "dependencies point downward and inward" applied correctly, and it is the right call given nothing under include/ needed eos/log.h. The new #include "eos/log.h" is a downward include into EoS core logging (§5 Core Services → Logging/trace), not upward into any product.

No tier boundary is crossed, no repository split is implied (§21.1 not in play), and nothing touches eBoot's TCB (§5.1, §8).

No check was weakened — the opposite. I read every hunk for this specifically. Five functions stop returning 0 for work they did not do; || true is removed from three commands and || echo … from two; the sha256sum fallback that substituted a flat digest for a Merkle root is deleted with a correct explanation; dv->verified is no longer set by create(), which had made every format look like a completed verification. Test fixtures move from fopen("w") to open(O_CREAT|O_EXCL, 0600), closing the three CodeQL sites. Nothing is skipped, xfailed, #if 0'd or deleted.

The contract change is properly justified. I re-ran the caller survey rather than taking it: git grep -nE "eos_(selinux|ima|busybox)_(install_to_rootfs|label_rootfs|sign_file)" across systems/, cmd/, examples/, services/, core/ and kernel/ at this head returns nothing outside linux_security.c and its header and test. No in-tree caller can break on the new -1. This also means findings 1 and 2 break nobody today — they are traps for the first real caller, which is why they are worth fixing before one exists rather than after.

The author's correction about the citation is right, and I am recording it as correct. .ai/security.md has no "Fail closed" heading, and the sentences "A verification step that cannot run must fail, not pass" and "A security step whose result is discarded is a finding even when the happy path is correct" are not in that file or anywhere in the repo. The rule that is there — .ai/security.md, Never: "Report a check as passing without running it" — carries the finding on its own. Misquoting a specification in a review is worth flagging back, and flagging it back was the right thing to do.

The design gap this PR keeps running into is already recorded as a proposal for eos#119 in .ai/autoreview/proposals/2026-09.md ("§14.1 governs which crypto a security component uses, but not how it invokes anything"). Finding 1 is a failure mode that proposal does not cover — success inferred from residual output state rather than from a discarded exit status — so I have appended a short addendum there rather than duplicating the proposal.

Proposed changes

In order, smallest first:

1. Finding 1, one line in eos_dmverity_create() before the popen:

    /* A previous call's hash must not be able to stand in for this one:
     * root_hash[0] is the only success signal this function has. */
    dv->root_hash[0] = '\0';

2. Finding 2, normalise the return:

    int rc = system(cmd);
    dv->verified = (rc == 0);
    return rc == 0 ? 0 : -1;

3. Add the two regression tests (struct-reuse for create, == -1 for verify). Both are pure additions to tests/test_linux_security_paths.c and need no new fixture.
4. Finding 3, the six missing struct-pointer NULL guards.
5. Finding 4, drop 2>/dev/null at the four sites whose exit status is now load-bearing.

Findings 1–3 are small enough to land in this PR. Finding 4 changes build-log output and could reasonably follow separately.

On merge order: the tests/CMakeLists.txt observation is correct and still current — #118 and #127 add the identical test_crypto_ed25519_loworder registration block at the identical anchor, and #126 adds a stray blank line at the same place. Whichever lands second conflicts. I have not verified that #125 is closed and #127 replaces it beyond the author's statement.

Not checked

• Windows. The #else arms now return -1 where several returned 0, for steps that genuinely cannot run there. I did not build for Windows and no test covers it. This remains the largest untested behaviour change in the PR, and the author says so in the body.
• veritysetup, setfiles, evmctl are all absent from my host (which returns nothing for each) — the same condition the author reports. So every assertion that depends on one of those tools succeeding does not discriminate in my run either; only the sentinel-file/side-effect assertions do. That is the right technique for this and it is why finding 1 needed a hand-written probe rather than a test to surface.
• Path traversal through rootfs_dir into eos_kaudit_install_to_rootfs() and the /init writer — acknowledged as out of scope by the author, and I agree is_path_safe is the wrong control for it. I did not assess it. It is a real open question and should not be lost.
• No fuzz harness for either predicate in tests/fuzz/. Pre-existing; not closed here; I did not fuzz them myself.
• The argv-vector rewrite the author defers is the correct larger fix and I did not attempt to evaluate a design for it. Two of the nine sites are pipelines and would need restructuring.
• The other seven system()/popen() security paths in eos/services/ were not swept. This review covered linux_security.c only.
• cppcheck / clang-tidy / CodeQL results I read from checks.txt (all pass); I did not run them locally.
• What I did run, and on what: git archive of the head into a temp directory, cmake -S . -B build then cmake --build build -j4 → exit 0; ctest --no-tests=error --output-on-failure → 40/40 passed; ./tests/test_linux_security_paths → 17/17 sub-tests PASS; the finding-1 probe linked against the built libeos_linux_security.a. GCC 15.2.0 (Ubuntu), x86_64 Linux, single configuration. The -Wformat-truncation= warning the author could not reproduce locally is confirmed fixed — my build emits 18 warnings, none of them in linux_security.c; they are in core/src/scheduler.c:189, services/pkg/eos_pkg.c (5), systems/src/rootfs.c (several) and tests/test_crypto_aes.c (2), all pre-existing and outside this diff. I did not run pytest tests/ -q (the author reports 15 passed) and I did not build any cross target.

---

Automated architecture review of 3768a14e36ac — scheduled, model claude-opus-5, checked against the EmbeddedOS Master Design v2.0. Advisory only: this reviewer never approves, requests changes, or merges. Reply here to discuss or push back — a wrong finding is a bug worth reporting.

[srpatcha]

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

head: 1a5039a author: Kartikey1306 ci: fail (2 of 26 — see finding 5, cause is external)

Verdict: Conforms, and the security direction is right. I re-derived the two claims the change turns on and both hold: is_path_safe now rejects backtick, newline, control characters and backslash and refuses NULL; is_word_safe is a genuine allowlist (isalnum() plus ._/+=:-, leading - refused); the fill-then-validate reordering in eos_busybox_configure closes the default path; and the caller survey is accurate — git grep across the head for the five contract-changed functions returns only the header, the implementation, the new test and docs/coverage/*.html, so nothing can break on the new -1. Nothing was weakened: every removed line in linux_security.c is a || true, an || echo, a discarded system() result, or the old if (!path) return 1. What remains is one step that still reports success without achieving its purpose, and a test summary that reports green over sub-tests that did not run.

Findings

#	Severity	File:line	Finding	Recommended fix
1	Medium	services/linux/src/linux_security.c:699-712	/init is written with fopen(path, "w") and never made executable, so it lands 0666 & umask — 0644 in practice. The kernel execs /init from an initramfs; a non-executable /init panics at Failed to execute /init. So the success path of eos_busybox_install_to_rootfs() still produces an initramfs that cannot boot, and returns 0 — while the new failure text two lines above says "cannot write %s; the initramfs has no /init" and the install branch says "its /init would not run". Pre-existing, but it is the same defect class the PR exists to close, on a line the PR modified: a step reporting success for work that did not achieve its purpose.	if (fchmod(fileno(fp), 0755) != 0) { EOS_ERROR(...); fclose(fp); return -1; } before writing, and extend the positive control to assert access(init, X_OK) == 0. Without the assertion the next refactor loses it again.
2	Medium	tests/test_linux_security_paths.c:36-45, main	The suite prints All linux_security path checks passed and exits 0 even when sub-tests did not run. have_tool("make")/("evmctl") and every if (!mkdtemp(dir)) { ... return; } return from the test function before PASS_IF_CLEAN, so the skipped case prints neither [PASS] nor [FAIL], failures is untouched, and the final line still claims everything passed. Nothing distinguishes "17 of 17 ran" from "11 of 17 ran" — including the 17/17 sub-tests PASS figure in the PR body, which is a count of what the author's host happened to be able to execute. .ai/security.md: "a security suite that silently collected nothing is a failed check reported as green." The [SKIP] markers themselves are right and were asked for; the missing piece is that nothing accounts for them.	Add static int skipped;, increment at each [SKIP], and print %d skipped in the summary; make the final line conditional on it (All … passed (N skipped)). Then have CI install make/evmctl/setfiles on one leg and fail that leg when skipped != 0, so the coverage claim is enforced somewhere rather than depending on the host.
3	Low	services/linux/src/linux_security.c:59-67 vs :91	is_word_safe explicitly refuses a leading - — "it turns make -C dir <defconfig> into an option rather than a target" — and is_path_safe does not. Every is_path_safe value is interpolated inside double quotes, so the shell is contained; quoting does not stop the invoked program from parsing the value as an option. setfiles -r "-Z" "$fc" "-Z", cp -r "--parents/"* …, make -C "--version" all reach the tool as options, from the same caller-controlled fields this change is hardening. Same defect class as the one the sibling predicate already guards, one layer up.	if (path[0] == '-') return 0; in is_path_safe; add "-r" and "--reference=/etc/shadow" to the HOSTILE table. A legitimate relative path can always be written ./-name.
4	Low	services/linux/src/linux_security.c:136, 175, 266, 294, 679 (and :110, :114, :125)	The truncation guards were added to eos_busybox_configure and eos_busybox_build only. The PR body says "truncation guards on every snprintf in both busybox functions", but eos_busybox_install_to_rootfs (:679) is a busybox function that builds a command from an unbounded const char *rootfs_dir with no check, and the four SELinux/IMA command builders are unchecked too. In the command cases truncation ends the string inside an open ", so sh reports a syntax error and the new non-zero return catches it — safe by an accident of quoting, not by a check. :110/:114/:125 are the ones not covered by that accident: they truncate path[1024] and hand the result to MKDIR/fopen, so a long rootfs_dir writes /etc/selinux/config somewhere other than where the caller asked (in practice fopen then fails, so the outcome is a confusing -1 rather than a wrong write).	Check the return of those snprintfs and return -1 on truncation, exactly as the two busybox functions now do. Either that, or narrow the body's claim to the two functions it is actually true of.
5	Low	CI	Cross-compile ARM64 kernel and the Full-stack integration summary that depends on it fail on this head. The cause is not in this PR. The failing step is Build eBoot (ARM64 qemu_arm64 board), and the errors are in the eBoot checkout: eBoot/include/eos_image.h:135 and :142 static-assert on offsetof(eos_image_header_t, reserved) / sizeof(…->reserved) and the struct has no reserved member (error: 'eos_image_header_t' has no member named 'reserved', run 33844971108). The workflow checks eBoot out at a floating master, so eBoot's trunk drifting breaks this repo's PRs — the same two jobs pass on eos#127, #129 and #135, which share #119's merge base and ran before the drift. eos#135 ("pin the eBoot and ebuild checkouts instead of floating on master") is the fix.	Nothing to do here beyond rebasing once #135 lands. Do not chase it in this branch, and do not read the red X as a verdict on this diff.

Architecture conformance

Conforms. §21 tier placement: services/linux is a Tier-1 eos core service and that is where a rootfs-provisioning helper belongs; nothing moved. §5.1 dependency direction: the one new link is target_link_libraries(eos_linux_security PRIVATE eos_core), which points inward within Tier 1, and PRIVATE is the correct choice — I checked, nothing under services/linux/include/ includes eos/log.h, so no consumer needs eos_core's include directories, and CMake still records the link requirement as $<LINK_ONLY:> for a static library. No upward include, import or link is introduced; no cross-repo path dependency (.ai/platform.md). §14.1 "use reviewed libraries, do not invent primitives" is untouched — this change is input validation, not cryptography.

The deeper architectural point — that a Tier-1 core service builds shell command strings from configuration data at nine call sites at all, and that string validation is a weaker guarantee than never involving a shell — is the author's own "left for a separate change" and is already recorded as a design proposal triggered by this PR (.ai/autoreview/proposals/2026-09.md, "§14.1 governs which crypto a security component uses, but not how it invokes anything", 2026-09-03), including the argv-vector rule, the allowlist-not-denylist rule, and the exit-status rule. No new proposal is appended for it, and it is not re-raised as a finding against this diff.

Proposed changes

1. fchmod(fileno(fp), 0755) on /init, checked, plus the X_OK assertion in the positive control. This is the one finding that changes what the shipped artifact does.
2. static int skipped; + %d skipped in the summary; then a CI leg with the tools installed that requires skipped == 0.
3. if (path[0] == '-') return 0; in is_path_safe, with two more HOSTILE rows.
4. Truncation checks on the five remaining command builders and the three path[1024] writes, or a narrower claim in the body.

None of these interact, and none of them touches the contract change, so they can land in any order. Findings 3 and 4 are the kind of thing that is cheaper now than after the argv-vector rewrite lands.

Not checked

• Nothing was built or executed. No compiler was invoked, ctest was not run, and the 40/40, 17/17 and pytest 15 passed figures are neither confirmed nor disputed. Everything above is read from the file contents at 1a5039a1 (fetched read-only into a private ref; the working tree was not touched) — labelled Observed, not Verified.
• The eleven-mutation table is not reproduced. Mutation testing is the right method here and the two self-reported non-discriminating tests are exactly the trap it exists to catch, but I have no independent evidence for any row of it.
• Windows: the #else arms now return -1 where several returned 0, and no test covers any of them. Already stated in the body under Not covered; recorded here, not re-raised.
• Path traversal through rootfs_dir into eos_kaudit_install_to_rootfs() and the /init writer — .. is not a shell metacharacter and is_path_safe permits it, so a rootfs_dir of /tmp/x/../../etc writes outside the rootfs with no shell involved. The author names this and explicitly excludes it. I did not assess it and it is still open, not fixed.
• No fuzz harness for either predicate (tests/fuzz/); pre-existing, not closed here.
• The other system()/popen() sites in services/ and eBoot/tools/ were not swept, so I cannot say this file is the only instance of the pattern.

---

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