[Kartikey1306]

Update: #144 has merged, so this now applies directly to master — there is no longer a parent to merge first. The stacking note below predates that.

Stacked on #144 (origin/master does not build without it). Review 9569226..0fced8b for this change: two commits, one file. Land #144 first.

Committed proof that the macro fails rather than skips (0fced8b). The negative control below was a one-off stubbed mkdtemp() at #144's head — evidence for this PR, but nothing that runs again. test_require_tmpdir_fails_rather_than_skips makes it repeatable: a probe calls REQUIRE_TMPDIR on a template whose parent directory does not exist (ENOENT for any user; a read-only directory would not do, root writes there and CI may run as root), in a forked child so the recorded failure cannot leak into the suite's own count, with the child's stderr captured over a pipe so the assertion is on the message class — [FAIL] naming mkdtemp(tmpl), and no [SKIP]. With the macro reverted to its old body (binary confirmed relinked) the run reports 4 check(s) failed, exit 1; with the fix, [PASS] and exit 0.

Closes #159

Problem

Twelve tests in tests/test_linux_security_paths.c opened with if (!mkdtemp(dir)) { fprintf(stderr, "[SKIP] mkdtemp failed\n"); return; } — fifteen mkdtemp() calls in all — and one mkdir() and one chmod() were handled the same way. On a read-only or full /tmp each test returned before its CHECK()s ran, failures stayed 0, the binary exited 0 and ctest was green. The review on #144 found it (finding 1, Low) as that PR added the twenty-first site; it is deferred here so the repair stays tight.

Change

The file's skip policy is kept for what it was written for — a missing build tool (make, evmctl, setfiles) or a host that ignores file modes means the case cannot be exercised, and [SKIP] is the honest answer. Those five sites are unchanged. A fixture the harness cannot create is the harness failing: REQUIRE_TMPDIR() counts it, names the file, line, template and errno, says which assertions did not run, and stops the test; FIXTURE_FAILED() does the same for the fixture mkdir() and chmod().

Verification

check	result
normal run	build clean; all linux_security path checks passed
negative control — stub mkdtemp() that always fails, linked in, at #144's head (old idiom)	exit 0, 12 [SKIP] lines, 6 [PASS] — green with twelve assertions silently gone
same stub, this commit	exit 1, 12 [FAIL] lines naming mkdtemp and EROFS, 6 [PASS]
the five environmental skips	unchanged: make ×2, evmctl, setfiles, running-as-root

A first attempt at the negative control redefined mkdtemp as a macro and collided with unistd.h's declaration; rc 127 was "no binary", not a result. The linked stub is the mutation that actually ran.

[Kartikey1306]

Added the committed test at 0fced8b — the mechanism was right; what was missing was something that runs every build and fails if the macro ever goes back to skipping.

test_require_tmpdir_fails_rather_than_skips: a probe calls REQUIRE_TMPDIR on /nonexistent-eos-parent-…/eos_XXXXXX, which mkdtemp() refuses with ENOENT for any user (a read-only directory would not discriminate — root writes there anyway, and CI may run as root). The probe runs in a forked child so the failure it records stays out of this process's count; the child's stderr comes back over a pipe, and the checks are on the message class: [FAIL], naming mkdtemp(tmpl), and no [SKIP] anywhere in it. The child exits 0 only if exactly one failure was recorded and the macro returned, so "continued past a failed mkdtemp" is caught as well.

Discriminates, measured at 0fced8b in this shell: macro reverted to its old [SKIP]-and-return body → binary confirmed relinked (mtime changed) → 4 check(s) failed, exit 1. Restored → [PASS] REQUIRE_TMPDIR counts a failed mkdtemp as a failure, reports it as [FAIL], and stops the test, exit 0, and the child's [FAIL] line is absent from the suite's own stderr. Full tree: ctest 43/43, pytest tests/ 47 passed. fork/pipe/waitpid are safe here because the target is already under if(UNIX).

Body updated: range is now 9569226..0fced8b, two commits, one file. Merge order unchanged — #144 first.

[srpatcha]

Review — eos#160 "test(linux_security): a fixture the harness cannot create is a failure, not a skip"

head: 0fced8b author: Kartikey1306 ci: pass — 29 green, 0 red; Create GitHub Release and assign skipped

Verdict: First look at 9569226..0fced8b (two commits, one file — tests/test_linux_security_paths.c, +102/−25) as the body asks; the other nine files belong to #144, reviewed separately at this exact base. The change is correct and the claims hold: I reproduced both negative controls from scratch, and both came out exactly as the body says. This is the right fix for .ai/security.md's fail-closed rule, and the committed probe is what makes it stay fixed. One gap: the sweep converted every [SKIP]-on-fixture-failure site, but two fixture open() calls in the same file never used [SKIP] — they silently ignore their result, and I proved both tests still report [PASS] with the fixture absent.

Findings

#	Severity	File:line	Finding	Recommended fix
1	Medium (P2)	tests/test_linux_security_paths.c:418-419, :514-515	Two fixture open() results are discarded, so two tests pass with their fixture missing — the same defect class this PR exists to close, in the file it just rewrote. Both sites are fd = open(..., O_CREAT|O_EXCL, 0600); if (fd >= 0) { (void)!write(...); close(fd); } — no CHECK, no failures++, no REQUIRE_. The PR enumerated the [SKIP] sites (15 mkdtemp, 1 mkdir, 1 chmod) and converted all of them; these two never printed [SKIP], so the sweep missed them. Proved by execution, not read: with both open() calls forced to fd = -1 and nothing else changed, the suite still prints [PASS] ima_sign_file reports failure when evmctl is absent and [PASS] selinux_label_rootfs reports a rootfs it did not label, then All linux_security path checks passed, exit 0. eos_ima_sign_file fails because the target file does not exist, and eos_selinux_label_rootfs fails because file_contexts does not exist — each assertion passes for a reason the test was not written to measure. The four (void)!write(...) calls at :419, :515, :640 and :701 discard their result the same way; a short or failed write leaves an empty fixture file behind an assertion that still passes.	CHECK(fd >= 0); if (fd < 0) { rmdir(dir); return; } at both sites — the pattern already used at :316-317 and :638-639 four lines away, so no new idiom is needed. For the writes, CHECK(write(...) == (ssize_t)n). That makes the file's own stated policy true of every fixture step in it, which is the claim the header comment at :41-55 now makes.
2	Low (P3)	tests/test_linux_security_paths.c:64-70	FIXTURE_FAILED()'s comment says "count it, say so, stop", and the macro does not stop — it counts and returns to its caller, which then has to return itself. All three call sites (:690, :731, and the two in test_require_tmpdir_fails_rather_than_skips) do so correctly, so nothing is wrong today. But its sibling REQUIRE_TMPDIR() five lines above does return, and the two macros read as a pair. The next person to add a fourth call site and trust the comment writes exactly the bug this PR is fixing.	Either make the comment accurate — "count it and say so; the caller must clean up and return" — or give it the return and drop the explicit ones. The comment is the cheaper change; the cleanup differs per site, which is presumably why it does not return.
3	Low (P3)	tests/test_linux_security_paths.c:812-815	The capture loop is while (total < sizeof out - 1 && (n = read(fds[0], ...)) > 0) total += n;. A read interrupted by a signal returns -1/EINTR, which ends the loop with a partial buffer and fails three of the four CHECKs below it — a flaky failure in a test whose whole purpose is that failures are real. Nothing in ctest sends signals, so this is latent rather than observed.	while (... && ((n = read(...)) > 0 || (n < 0 && errno == EINTR))) { if (n > 0) total += n; }, or wrap in a retry. errno.h is already included by this commit.

Not findings, recorded so they are not mistaken for uncovered:

• The committed probe discriminates. With REQUIRE_TMPDIR reverted to its old
  [SKIP]-and-return body and nothing else changed, the suite reports 4 check(s) failed and
  exits 1 — the author's claim, reproduced exactly, including the count.
• _exit() in the forked child is right, and matters: printf output is fully buffered when
  ctest captures stdout, and a normal exit() in the child would flush the parent's inherited
  buffer and duplicate every [PASS] line printed before the fork. The clean verbose run
  confirms no duplication.
• Choosing a non-existent parent directory over a read-only one is the right call and the
  comment says why: CI may run as root, and root writes into a read-only directory. ENOENT
  discriminates for every user.
• The probe fails safe if /nonexistent-eos-parent-3f9c1a ever exists. mkdtemp would then
  succeed, the probe would print REQUIRE_TMPDIR continued past a failed mkdtemp, and
  CHECK(strstr(out, "mkdtemp(tmpl)")) would fail. The test breaks loudly rather than passing
  vacuously, which is the correct direction.
• The five environmental [SKIP]s are genuinely untouched — make ×2, evmctl, setfiles,
  running-as-root — and they are the right five: each names a dependency whose absence means the
  case cannot be exercised, not a harness that failed. grep -n "SKIP" on the head returns
  exactly those five plus three comment lines and the probe's own assertion.
• fork/pipe/waitpid are safe here. tests/CMakeLists.txt:362 wraps this target in
  if(UNIX), so the MSVC leg never compiles it. Confirmed at this head.
• No other test in the tree has this idiom. mkdtemp appears in exactly one file under
  tests/, and no other .c or .py there skips on a failed fixture step. Unlike the caller
  duplication in ebuild#144, this defect class does not need a second sweep.
• Nothing was weakened: no test disabled, no assertion removed, no lint loosened, no || true.
  The commit adds one test and converts 17 silent returns into counted failures.

Architecture conformance

Conforms. §21 places eos in Tier 1 — Foundation; tests/ is inside the owning repo and
§21.1 is not engaged. §5.1's dependency law is not touched — the diff adds <sys/wait.h> and
<errno.h>, both libc, and no product-repo include, link line or manifest entry. §8's eBoot TCB
rules are not engaged.

What this serves is §28's evidence model directly: test_linux_security_paths is the functional
evidence behind linux_security's "Implemented" status, and until this commit twelve of its
eighteen cases could return before asserting anything while the binary exited 0 and ctest
went green. .ai/security.md's fail-closed rule — "a verification step that cannot run must
fail, not pass" — is the local statement of it, and this is that rule applied one level below
where it is usually applied.

Proposal appended. §28 and the existing draft §28.2 govern the runner — a suite that
collects zero tests, a job that cannot fail, a gate that never runs — and are silent on a test
that runs, returns early having asserted nothing, and reports success. --no-tests=error does
not reach it. See proposals/2026-09.md, 2026-09-14, "Addendum: a test that runs and asserts
nothing satisfies every rule §28.2 has".

Proposed changes

1. CHECK the two fixture open()s at :418 and :514, and the four writes
   (:419, :515, :640, :701). Same two-line pattern as :316-317.   (finding 1)
2. Fix FIXTURE_FAILED's comment, or give the macro its return.    (finding 2)
3. Retry the capture read on EINTR.                               (finding 3)

None of these blocks the PR. Finding 1 is the one worth doing before merge, because the header
comment this commit adds at :41-55 now claims a policy the file does not quite hold to.

No fix PR opened. All three findings are Medium or Low; the brief's autofix rule is
High-only. They are also in a file as this branch rewrites it, so a branch cut from
origin/master — which is what fix-start.sh produces — would be patching a different body.

Verification performed for this review

The eos checkout has a clean working tree (git status --short empty, before and after)
and was not modified — no worktree, checkout, stash or reset. The PR head was fetched
read-only into refs/autoreview/pr160. Everything below ran against git archive | tar -x into
two temporary directories outside the repository — one at 0fced8b2, one at the base
9569226 for the controls. Nothing was committed or pushed.

Check	Result
cmake -B build/host -G Ninja -DCMAKE_BUILD_TYPE=Release -DEOS_BUILD_TESTS=ON -DEOS_PRODUCT=vbox_test	PASS
cmake --build build/host --parallel 4	PASS, no warnings in the changed file
ctest --output-on-failure --no-tests=error --timeout 120 --parallel 4	PASS — 100%, 43/43, 0 failed
ctest -R test_linux_security_paths --verbose	19 [PASS], 0 [SKIP], 0 [FAIL], including [PASS] REQUIRE_TMPDIR counts a failed mkdtemp as a failure…. The new test executes; it is not a function nobody calls
Control A — REQUIRE_TMPDIR reverted to the old [SKIP]-and-return body, target relinked	FAIL, exit 1, 4 check(s) failed — the four CHECKs in test_require_tmpdir_fails_rather_than_skips, by line: :820 exit status, :821 [FAIL] present, :822 mkdtemp(tmpl) present, :823 no [SKIP]. The author's claimed count, exactly. Restored; green again
Control B — mkdtemp() forced to fail (LD_PRELOAD, returns NULL/EROFS), this head	exit 1, 12 [FAIL] lines naming mkdtemp, 0 [SKIP], 7 [PASS]
Control B — same stub, base 9569226 (old idiom), rebuilt from a separate export	exit 0, 0 [FAIL], 12 [SKIP], 6 [PASS] — green with twelve assertions silently gone. The defect, reproduced
Control C — both fixture open()s forced to fd = -1 (finding 1)	exit 0, All linux_security path checks passed, and [PASS] printed for both ima_sign_file reports failure when evmctl is absent and selinux_label_rootfs reports a rootfs it did not label. Restored; green again
grep -n "SKIP" tests/test_linux_security_paths.c at this head	five printf("[SKIP] …") sites, all environmental (make ×2, evmctl, setfiles, root); the rest are comments and the probe's own assertion
grep -rln "mkdtemp" --include="*.c" tests/	one file — this one. No second sweep needed
tests/CMakeLists.txt:362	if(UNIX) wraps the target; the fork/pipe/waitpid addition cannot reach the MSVC leg
git diff --stat 9569226 0fced8b2	one file, +102/−25 — the body's scope claim is accurate
CI on this head (checks.txt)	29 pass, 0 fail, incl. Build & Test (Linux x86_64), Build & Test (Windows MSVC), Host Tests (x86_64), Cross-compile ARM Cortex-M4, Cross-compile ARM64 kernel, QEMU Boot Test, QEMU ARM64 simulation (13 TCs), Static Analysis (cppcheck + clang-tidy), CodeQL, Analyze (C/C++)
PR metadata	mergeable: MERGEABLE, mergeStateStatus: BLOCKED, reviewDecision: REVIEW_REQUIRED

Not checked

• pytest tests/ — NOT RUN. The author reports 47 passed; the Python legs are green in CI
  and this commit changes no Python, so nothing in it is exposed by that gap. Unverified locally.
• Cross-compilation and MSVC — NOT RUN locally. Only the host x86_64 build was configured.
  The ARM Cortex-M4, ARM64 and Windows legs are green in CI and were not reproduced. The MSVC
  leg matters here specifically because of the new fork/pipe/waitpid; I verified the
  if(UNIX) guard by reading tests/CMakeLists.txt, not by compiling on Windows.
• QEMU simulation and cppcheck/clang-tidy — NOT RUN locally. Green in CI; not reproduced.
• Control B used LD_PRELOAD, not the author's linked stub. Same effect on the call, a
  different mechanism; a statically-resolved mkdtemp would not be intercepted this way. The
  numbers matched the body's on both sides, which is the point of the control.
• The real trigger — a read-only or full /tmp — was not reproduced. Every control forced
  the failure at the call rather than creating the filesystem condition.
• The EINTR path in finding 3 was not provoked. It is read from the loop, not observed.
• The #144 half of the 10-file diff is not reviewed here — reviewed separately at head
  95692266, which is this PR's base. If #144 changes, the 43/43 figure no longer describes
  what will merge.
• mergeStateStatus: BLOCKED — REVIEW_REQUIRED plus the #144 stack. Land #144 first.

---

Automated architecture review of 0fced8b2c866 — 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 three addressed at c57fad4.

Finding 1 — both fixture open()s (:421, :520) now CHECK(fd >= 0) and return with cleanup, the pattern from :316-317; all four fixture writes CHECK their byte count instead of (void)!-discarding it. Reproduced your probe at this head, in this shell: both open() calls forced to -1, binary confirmed relinked → [FAIL] …:422: fd >= 0, [FAIL] …:521: fd >= 0, 2 check(s) failed, exit 1 — where you measured exit 0. Restored → All linux_security path checks passed, exit 0. The header comment's policy is now true of every fixture step in the file.

Finding 2 — comment made accurate rather than adding a return: "count it and say so; it does not return — the cleanup differs per site — so the caller must clean up and return itself; REQUIRE_TMPDIR does return because an uncreated directory needs no cleanup." Your read of why they differ was right, so the comment says it.

Finding 3 — the capture loop retries on EINTR and breaks on any other error or EOF.

Verified at c57fad4: ctest 43/43, pytest tests/ 47 passed. One thing worth recording since you grade artefacts: my first full-suite run after the probe showed 42/43 — the restore had not relinked the binary, so the probe was still in it. touch + rebuild, 43/43. The commit message figure was written after that.

Range for this round: 0fced8b..c57fad4. Merge order unchanged — #144 first.

[srpatcha]

Reviewed. Approve (test-only). Nit: in the two-directory tests, if the second REQUIRE_TMPDIR fails, the first directory is not removed.