[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.

Fixes #161

Stacked on #144; review 9569226..f58ee19 for this change alone -- four commits; f58ee19 clears the CodeQL check-run (three high alerts on the new code: the POSIX walk now acts on directory descriptors with unlinkat/openat(O_NOFOLLOW) instead of lstat then unlink, and the two fixture files are created 0600) (the third, a one-line test fix for the MSVC leg: the db records now use the platform path separator). c3ba938 is the fix; 3e2b9a8 answers the review's four findings (services/pkg/eos_pkg.c, tests/test_pkg_install_paths.c, CHANGELOG.md). c3ba938 alone was four files: services/pkg/eos_pkg.c, tests/test_pkg_install_paths.c (new), its registration in tests/CMakeLists.txt, one CHANGELOG line at the end of ### Fixed. Merge #144 first. Independent of #156 and #158 (the registration sits beside test_pkg_trust_anchor, not where #156 adds test_shell_cmd).

The defect

eos_pkg_install() writes the binary to apps_dir/<package_id>/<name> with both fields taken raw from the package header, and the signature covers the binary alone (eos_pkg_check_signature(hdr.signature, binary_data, hdr.binary_size)), so both are attacker-chosen even for a genuinely signed package:

• package_id = "../escape": install returns 0, prints installed 'payload' v1.0.0, and leaves apps_dir/../escape/payload at mode 0755. Reproduced on 9569226 by the new test's negative control (below).
• A field that fills all 64 bytes has no terminator; verify/install/update print and join it with %s, reading past the header struct.
• eos_pkg_remove() hands install_path to rm -rf "..." / rmdir /s /q "..." through system(), so a " in package_id reached a shell.

The fix

eos_pkg_field_is_component(): a field is a single path component -- terminated, non-empty, [A-Za-z0-9._-] only, neither . nor ... eos_pkg_check_header_names() applies it to both fields and is called from verify(), install() and update():

• after the signature check, so an unsigned package is still reported as unsigned (the fix(pkg): give eos_pkg a real trust anchor, or refuse to verify #120 refuse-unless-anchored gate is unchanged and fires first), and before the first snprintf that reads the fields;
• update() now calls eos_pkg_verify() before looking the id up, so nothing prints or compares a package_id the package was not entitled to carry.

Signing the header itself (capabilities, version, names, resources) is a package-format change; filed as #162 rather than folded in here. This PR closes the reachable filesystem and shell paths with the format as it is.

Verification

Check	Result
cmake -B build -DEOS_BUILD_TESTS=ON → cmake --build	PASS; no new warnings. With clang here eos_pkg.c compiles clean at the base and at this head; the review counted 8 pre-existing GCC warnings in the file at the base (-Wformat-truncation, -Wstringop-truncation, -Wunused-result on system at stop()), and the tree-wide GCC count was 89 at both. The -Wunused-result one is gone at 3e2b9a8 with the system() call it was on; the other seven are untouched pre-existing snprintf/strncpy sites. eos_eapp builds with -Wall -Wextra -Wpedantic.
ctest --test-dir build	44/44 (43 on the base + test_pkg_install_paths, now 10 tests)
tests/test_pkg_install_paths	8/8: traversal via package_id; separator in name (/ and \); . and .. in either field; 13 shell-metacharacter ids (x" ; echo pwned ; ", x$HOME, x`id`, newline, tab, ...); unterminated 64-byte fields through verify and install; empty fields; the update path; and the control -- the same signed payload under org.eos.test.paths / payload-1.0_x installs and lands where it should
Negative control: git show 9569226:services/pkg/eos_pkg.c > services/pkg/eos_pkg.c, rebuild, rerun	first test fails at tests/test_pkg_install_paths.c:182: eos_pkg_install(&db, EAPP_PATH) != 0 after printing installed 'payload' v1.0.0, and test_install_paths_escape/payload (-rwxr-xr-x, 1 byte) is left beside apps_dir
pytest tests/unit -q (test_cmake_test_registration.py among them)	43 passed
Line endings	all four files LF on the base and LF here

The fixture is created and removed in the working directory, the way test_pkg_trust_anchor.c does it; the POSIX branch creates the package 0600, and the Windows branch uses _rmdir from <direct.h> for cleanup.

Review follow-up (3e2b9a8)

Finding	What changed
1 (P1) -- a db record still reached rm -rf "%s" / pkill -f "%s" through system()	Both parts. (a) eos_pkg_load_db() holds every record to the rules install enforces: name and package_id must be single path components, and install_path must be exactly apps_dir/<package_id>; a record that fails is dropped with a message (the next save writes the db without it), so remove and stop never see it. (b) Nothing in the file reaches a shell any more, without waiting for #156: eos_rmdir_recursive() is a directory walk in C on both platforms (opendir/lstat/unlink/rmdir; FindFirstFileA/DeleteFileA/RemoveDirectoryA), symlinks unlinked rather than followed, and remove() refuses a path other than the package's own install directory before walking it; stop() hands the name to pkill -f / taskkill /IM as an argv element through posix_spawnp() / _spawnvp(). Tests: your legacy record (install_path = apps/x" ; touch sentinel ; echo "), one with a shell escape in name, and one well-formed, saved with eos_pkg_save_db() and reloaded -- two dropped, one kept, remove/stop refuse the dropped ones, no sentinel; and a removal that takes down a nested directory and a file named with backticks while a sibling directory survives, plus a record whose install_path is the apps directory itself refused at removal. Against c3ba938 the first fails at the record count.
2 (P2) -- update() validated verify()'s read and used its own	eos_pkg_check_header_names(&hdr) right after update()'s own fread/fclose, before the lookup, the message, remove() and the final printf; verify() stays where it is for the signature. The overstated comment is replaced with what is now true.
3 (P3) -- install() printed hdr.package_id before validating it	Hoisted to right after the magic/version checks, as you suggest -- in install() nothing is said about the package before the signature except that line, so the verify-side ordering argument costs nothing here.
4 (P3) -- the warnings claim	Reworded in the table above: no new warnings; clang is clean at both ends, the 8 GCC warnings you counted are pre-existing, and the -Wunused-result on system at stop() is gone with the call.

CodeQL (check-run, not the Analyze job): three high alerts on 3c72b23's new code -- a TOCTOU in the walk (lstat then unlink/recurse on the same path) and two fopen("wb") fixtures. Cleared in f58ee19: the walk consumes an open directory descriptor, unlinkat() first (removes a file or a symlink, never follows), descends only on EISDIR/EPERM through openat(O_DIRECTORY|O_NOFOLLOW), removes with unlinkat(AT_REMOVEDIR); nothing is stat'ed. The test plants a symlink from inside the package to a sibling directory and asserts the link goes and the sibling stays. Fixtures: open(O_CREAT, 0600) + fdopen(), the class fixed in bba834a and 8dca787.

[srpatcha]

Review — eos#163 "fix(pkg): a .eapp header names files, it does not name paths"

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

Verdict: First look at 9569226..c3ba938 (one commit, four files — services/pkg/eos_pkg.c
+72/−5, tests/test_pkg_install_paths.c new +321, its tests/CMakeLists.txt registration, one
CHANGELOG line) as the body asks; the other eight files belong to #144, reviewed separately at
this exact base. The core fix is correct and the headline claim reproduces: I built the
tree, ran ctest (44/44), and the negative control fails at the exact file:line the body
names, printing installed 'payload' v1.0.0 and Path: test_install_paths_apps/../test_install_paths_escape.
eos_pkg_field_is_component() is a well-chosen whitelist and the placement argument — after the
signature so an unsigned package still reports as unsigned — is right.

But the validation guards the ingress and three consumers of the same fields are outside it.
The strongest is the one I proved by execution at this head: a package record loaded from the
database still reaches system(). I built a db whose install_path contains a double quote,
called eos_pkg_remove(), and touch /tmp/PWNED_163 ran. That is the same shell path this
PR's comment says it closed, entered from persisted state instead of from the header.

Findings

#	Severity	File:line	Finding	Recommended fix
1	High (P1)	services/pkg/eos_pkg.c:78-88 (eos_rmdir_recursive), reached from :816; also :1099 and :1106 (eos_pkg_stop); ingress eos_pkg_load_db at :415-468	A db record still reaches a shell. Proved by execution at this head. eos_pkg_load_db() validates magic, version, count ≤ EAPP_MAX_PACKAGES and truncation — and nothing about the strings inside each eapp_package_t. eos_pkg_remove() then hands pkg->install_path to rm -rf "%s" / rmdir /s /q "%s" through system(), and eos_pkg_stop() hands pkg->name to pkill -f "%s" / taskkill /IM "%s". Measured: a db record with install_path = apps/x" ; touch /tmp/PWNED_163 ; echo ", saved with eos_pkg_save_db(), reloaded by eos_pkg_init(), then eos_pkg_remove(&db, "legacy") → eos-pkg: removed 'payload' v0.0.0, exit 0, and /tmp/PWNED_163 exists. The precondition is a db that already contains such a record, which is exactly what the pre-fix code this PR is written against could write: install_path is built at :771 from an unvalidated package_id. So any device that ran eos-pkg install before this commit carries the record, and remove executes it afterwards. Rated High, not Critical: it is not an eBoot TCB break, a committed secret, silent data loss, or missing authorization on user data — the enumerated Critical cases in .ai/reviewer.md. It is arbitrary command execution as whoever runs eos-pkg, from data at rest, and .ai/security.md is explicit that a parser's severity is not rounded down because the path looks narrow.	Two parts, and the second is the durable one. (a) Run the same check on load: eos_pkg_load_db() should reject or drop a record whose name/package_id is not eos_pkg_field_is_component(), and whose install_path is not under db->apps_dir. That closes the legacy record without a format change. (b) Stop interpolating into system() at all. eos#156 is open and adds exactly the mechanism for this — core/include/eos/shell_cmd.h and core/src/shell_cmd.c, "one checked builder" — but its file list does not include services/pkg/eos_pkg.c, so these three call sites are covered by no open PR. Routing them through shell_cmd once #156 lands, or replacing eos_rmdir_recursive with a real nftw/FindFirstFile walk, removes the class rather than the instance.
2	Medium (P2)	services/pkg/eos_pkg.c:846-876	update() validates a different read of the file than the one it uses. It fopens, reads its own hdr, fcloses, then calls eos_pkg_verify(eapp_path) — which opens the file again and validates its copy of the header. Everything after that uses update()'s own hdr: eos_pkg_find(db, hdr.package_id) at :867, the '%s' is not installed message at :869-870, eos_pkg_remove(db, hdr.package_id) at :881, and the final printf at :892. If the file changes between the two fopens, none of those are covered — and the comment the commit adds at :861-863 claims they are: "nothing below prints or compares a package_id the package was not entitled to carry." The window is real but narrow (an attacker needs write access to the .eapp path while update runs), and update() already re-opened the file twice before this PR, so the race is not new — the overstated comment is.	One line, no race analysis needed: if (eos_pkg_check_header_names(&hdr) != 0) return -1; immediately after update()'s own fread/fclose at :857. Then update()'s copy is validated whatever happens to the file. Keep the verify() call where it is; it earns its place for the signature.
3	Low (P3)	services/pkg/eos_pkg.c:628-632	install() prints hdr.package_id with %s before validating it. The already-installed guard fires 68 lines above eos_pkg_check_header_names(). The comment at :692-694 says the check runs "before the first snprintf that would read them", which is true — the fprintf is not an snprintf. A package_id filling all 64 bytes with no terminator is one of the three defects the PR names, and %s on it walks past package_id[64] into ver_major, capabilities, arch_count, supported_archs[48] until it meets a zero byte. It stays inside the 303-byte eapp_header_t in practice, so this is a malformed print, not an out-of-bounds read of the object. Reachability is narrow, and gets narrower with this PR: eos_pkg_find() at :373-383 uses strncmp(..., EAPP_MAX_NAME), which is correctly bounded and does not over-read, so the fprintf only executes when an installed record matches all 64 bytes — which, once every install goes through the new check, it cannot. It remains reachable against a legacy db, i.e. under finding 1's precondition.	Move eos_pkg_check_header_names(&hdr) up to immediately after the magic/version checks at :622-627, and keep nothing else. The author's reason for putting it after the signature — that an unsigned package must report as unsigned — is sound for verify(), where the message is the product. In install() nothing is printed about the package before the signature check except this line, so hoisting it costs nothing and covers the fprintf too. If the ordering must stay, use "%.*s", (int)sizeof(hdr.package_id), hdr.package_id.
4	Low (P3)	PR body, Verification table row 1	"PASS; no warnings in the touched files (eos_eapp builds with -Wall -Wextra -Wpedantic)" is not what the build produces. services/pkg/eos_pkg.c emits 8 warnings at this head: -Wunused-result on system at :1106, four -Wformat-truncation at :703, :717, :743, :751, three -Wstringop-truncation at :769-771. The flags claim is accurate — services/pkg/CMakeLists.txt:17-21 does set -Wall -Wextra -Wpedantic for eos_eapp. The substance of the claim holds and I verified it: the identical eight warnings appear at the base 9569226 at lines :1039, :639, :653, :679, :687, :705-707, and the whole-tree count is 89 at the base and 89 at this head — the PR adds none. Recorded per brief item 5 for the wording, which says something stronger than what was measured.	Say "no new warnings in the touched files; the 8 pre-existing ones in eos_pkg.c are unchanged", and note that -Wunused-result on system at :1106 is finding 1 wearing a compiler's hat.

What this PR gets right, because it is most of the change and should not be lost in four
findings:

• eos_pkg_field_is_component() is a whitelist, not a blacklist of dangerous characters,
  which is the only version of this check that stays correct as consumers change. It handles all
  four cases in one place: unterminated (n == field_sz), empty (n == 0), ./.., and any
  byte outside [A-Za-z0-9._-] — which covers /, \, ", $, backtick, newline and tab
  without naming any of them.
• Placing the check after the signature in verify() is the right call and the comment says
  why: an unsigned package must be reported as unsigned, and the #120 refuse-unless-anchored gate
  still fires first. I confirmed the order at :568-578 — magic, version, binary_size,
  seek, malloc, exact-size fread, SHA-256, Ed25519, then names, then the first
  printf of either field.
• Not folding in the header-signing change is the right scope call, and #162 exists for it.
  Signing capabilities and the resource fields is a package-format change and belongs behind
  §23.2's migration policy, not in a path-traversal fix.
• The new test is a real functional test with a real negative control, not a mock.

Architecture conformance

Conforms. §21 places eos in Tier 1 — Foundation; services/pkg/ is inside the owning repo
and §21.1 is not engaged. §5.1's direction-of-dependency law holds: eos_pkg.c includes
<eos/crypto.h> and <ed25519.h>, both downward, and the diff adds no include, link line or
manifest entry — target_link_libraries(eos_eapp PUBLIC eos_crypto) is unchanged. §8's eBoot
TCB rules are not engaged; this is the application package manager, not boot.

What it serves is §10.1's component contract — Integrity: hashes, signatures and provenance —
and .ai/platform.md's rule that permissions and resource caps in a manifest are enforced, not
merely recorded. This PR is the narrower neighbour of that rule: a header field that names a
filesystem location must be constrained to what it claims to be. §11's artifact table lists
.eapp as the application package, and §14.1 asks for a documented threat model for package
distribution — which is precisely the gap the PR's own #162 names, since the signature covers
the binary and not the header that describes it.

No new proposal appended. The design gap this sits on — §11 having no provenance rule, so a
package's metadata is only as trustworthy as the document naming it — is already written up as
the 2026-09-03 proposal "§11 has no provenance rule, so a package's checksum is only as
trustworthy as the document that names its URL", and the signed-header question is tracked as
eos#162. Nothing at this head reveals a further gap in the master design; findings 1–3 are
code-level, not design-level.

Proposed changes

Before merge, both one-liners:
  eos_pkg.c:857   eos_pkg_check_header_names(&hdr) on update()'s own header  (finding 2)
  eos_pkg.c:622   hoist the same call above the already-installed guard      (finding 3)

Before merge or immediately after, not a one-liner:
  eos_pkg_load_db()  reject a record whose name/package_id is not a component
                     and whose install_path is not under apps_dir            (finding 1a)

Separately, and the one that ends the class:
  eos_rmdir_recursive() and eos_pkg_stop()'s two system() calls route through
  eos#156's core/src/shell_cmd.c, or stop shelling out entirely   (finding 1b)

Housekeeping:
  PR body row 1 -> "no new warnings"                                         (finding 4)

No fix PR opened. Finding 1 is High, which is inside the brief's autofix rule, and unlike
the other PRs in this run the defect does exist on origin/master — eos_rmdir_recursive()
and both eos_pkg_stop() system() calls are unchanged there — so fix-start.sh would have
something to patch. I am not opening one anyway, for two reasons the brief gives directly.
First, the fix is a refactor: routing three call sites through a command builder, or
replacing rm -rf with a directory walk, is not "an unchecked return, a wrong CMake option
name". Second, eos#156 is open and owns this mechanism — it adds core/src/shell_cmd.c for
exactly this purpose — and the brief says that where an existing PR covers the ground, say so in
the review and open nothing. What #156 does not cover is services/pkg/eos_pkg.c; its file
list is backends and services/linux, so someone has to extend it or follow it, and saying that
here is the useful contribution. Finding 1a (validate on load) is small enough to fix, but it
belongs in this PR, next to the check it extends, not in a competing branch.

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/pr163. Everything below ran against git archive | tar -x into
temporary directories outside the repository — one at c3ba9386, one at the base 9569226 for
the controls, and a third throwaway for the injection harness. 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
ctest --output-on-failure --no-tests=error --timeout 120 --parallel 4	PASS — 100%, 44/44, 0 failed. Matches the body's "44/44" exactly (43 at the base + the new suite)
ctest -R test_pkg_install_paths --verbose	PASS, 8 [PASS] — matches the body's "8/8". The suite executes; it is not registered-and-unbuilt
pytest tests/unit -q (CPython 3.12, pyyaml)	PASS — 43 passed in 0.34s. Matches the body
Negative control — git show 9569226:services/pkg/eos_pkg.c restored, target rebuilt	FAIL, as claimed, at the exact line: [FAIL] tests/test_pkg_install_paths.c:182: eos_pkg_install(&db, EAPP_PATH) != 0, preceded by eos-pkg: installed 'payload' v1.0.0 (x86_64) and Path: test_install_paths_apps/../test_install_paths_escape. Restored; green again
Finding 1, executed at this head — db record with install_path = apps/x" ; touch /tmp/PWNED_163 ; echo ", saved via eos_pkg_save_db(), reloaded via eos_pkg_init(), then eos_pkg_remove()	loaded count=1 install_path=[apps/x" ; touch /tmp/PWNED_163 ; echo "], eos-pkg: removed 'payload' v0.0.0, exit 0, and /tmp/PWNED_163 was created. Cleaned up afterwards
eos_pkg_find() over-read check	Not a defect — :380 is strncmp(..., EAPP_MAX_NAME), correctly bounded. Only the fprintf at :629 is unbounded (finding 3)
eos_pkg_load_db() input validation	magic, version, count ≤ EAPP_MAX_PACKAGES, per-record truncation — all present and correct. No validation of any string field
grep -n "system(|popen(" services/pkg/eos_pkg.c	four sites: :81, :85 (eos_rmdir_recursive), :1099, :1106 (eos_pkg_stop). eos_pkg_run at :1058 correctly uses execv with an argv array, not a shell
Warnings, base vs head	identical set, identical whole-tree count (89 → 89). Eight in eos_pkg.c on both sides, line numbers shifted by the insert. The PR adds none — finding 4 is the wording, not the build
services/pkg/CMakeLists.txt:17-21	eos_eapp does build with -Wall -Wextra -Wpedantic on GNU/Clang and /W4 on MSVC, as the body says
Open PRs touching services/pkg/eos_pkg.c	only this one. eos#156's file list is backends, core/shell_cmd, services/linux and tests — it does not reach eos_pkg.c
git diff --stat 9569226 c3ba9386	4 files, +399/−5 — 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

• Finding 2's race was not raced. The TOCTOU between update()'s own fread and
  eos_pkg_verify()'s re-open is read from :846-867, not demonstrated by swapping the file
  mid-call. The one-line fix makes the race question moot, which is why I did not build a
  harness for it.
• Finding 3's over-read was not provoked. With the fix in place I could not construct a db
  record whose package_id fills 64 bytes, because the new check prevents one being written —
  which is the point. It is read from :628-632, and its reachability is stated as conditional
  on a legacy db, not asserted.
• Windows — NOT BUILT. rmdir /s /q "%s" and taskkill /IM "%s" in finding 1 are the MSVC
  arms; only the POSIX arm was compiled and executed. cmd.exe quoting differs from sh
  quoting, so whether the same record yields command execution there is Unknown — I did not
  test it and am not asserting it. The _rmdir/<direct.h> cleanup the body describes in the
  new test is likewise unexercised. The Build & Test (Windows MSVC) leg is green in CI.
• Cross-compilation, QEMU, cppcheck/clang-tidy — NOT RUN locally. All green in CI, none
  reproduced. Note that neither cppcheck nor clang-tidy flagged finding 1 on this head, and the
  -Wunused-result on system at :1106 is present in the build and not treated as an error.
• GCC version differs from CI. My host GCC is newer than CI's gcc-12, so the warning set
  is not guaranteed identical to the one the green legs saw. The base-vs-head comparison was
  made with the same compiler on both sides, which is what finding 4 rests on.
• No fuzzing of the header. tests/fuzz/ exists and is opt-in (-DEOS_ENABLE_FUZZING=ON,
  Clang only); I did not enable it, and there is no fuzz target for eapp_header_t.
  .ai/security.md asks for fuzz coverage on externally reachable parsers, and the .eapp
  header is one. Not raised as a finding because it is not this PR's to add, but it is the
  obvious next thing after #162.
• hdr.binary_size is unbounded before malloc() at :540 and the install() equivalent
  — a 4 GiB header field forces a 4 GiB allocation attempt before the read fails. Pre-existing,
  untouched by this diff, not raised as a finding; recorded so it is not mistaken for reviewed.
• The #144 half of the 12-file diff is not reviewed here — reviewed separately at head
  95692266, which is this PR's base. If #144 changes, the 44/44 figure no longer describes
  what will merge.
• mergeStateStatus: BLOCKED — REVIEW_REQUIRED plus the #144 stack. Land #144 first.

---

Automated architecture review of c3ba93860ef2 — 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 four taken, in 3e2b9a8 (range now 9569226..3e2b9a8; the body has a "Review follow-up" table). The db-record execution is the half I had left open — thank you for running it rather than reading it.

Finding 1 — a db record still reached a shell. Both parts. (a) eos_pkg_load_db() now holds every record to the rules install enforces: name and package_id must be single path components, and install_path must be exactly apps_dir/<package_id>, the string install builds; a record that fails is dropped with a message and the next save writes the db without it, so remove() and stop() never see it. (b) The durable half, done now rather than after #156: nothing in eos_pkg.c reaches a shell any more. eos_rmdir_recursive() is a directory walk in C on both platforms (opendir/lstat/unlink/rmdir; FindFirstFileA/DeleteFileA/RemoveDirectoryA), symlinks unlinked rather than followed, and remove() refuses a path other than the package's own install directory before walking it; stop() hands the name to pkill -f / taskkill /IM as an argv element through posix_spawnp() / _spawnvp(). I did not route these through shell_cmd because with no shell line there is nothing for it to check. Tests: your legacy record (install_path = apps/x" ; touch sentinel ; echo "), one with a shell escape in name, and one well-formed, saved with eos_pkg_save_db() and reloaded through eos_pkg_init() — two dropped, one kept, remove/stop refuse the dropped ones, no sentinel; and a removal that takes down a nested directory and a file named with backticks while a sibling directory survives, plus a record whose install_path is the apps directory itself refused at removal. Against c3ba938 the first fails at the record count.

Finding 2 — update() validated a different read than it used. eos_pkg_check_header_names(&hdr) right after update()'s own fread/fclose, before the lookup, the message, remove() and the final printf; verify() stays where it is for the signature, and the comment says what is now true rather than more.

Finding 3 — install() printed hdr.package_id first. Hoisted to right after the magic/version checks, as you suggest; in install() nothing is said about the package before the signature except that line, so the ordering argument I gave for verify() costs nothing here.

Finding 4 — the warnings claim. Reworded: no new warnings. Clang here compiles eos_pkg.c clean at both the base and this head; I could not run GCC on this host, so your count stands as the GCC figure — 8 pre-existing, 89 tree-wide at both ends — and the -Wunused-result on system at stop() is gone at 3e2b9a8 with the call it was on.

ctest 44/44, test_pkg_install_paths 10/10, pytest tests/unit 43.

[Kartikey1306]

CodeQL's check-run (the "new alerts in code changed by this PR" one, not the Analyze job) flagged three high alerts on 3c72b23's new code; cleared in f58ee19 (range now 9569226..f58ee19).

• TOCTOU in the POSIX walk — lstat(child) then unlink(child) / recurse on the same path. Rather than narrow the window, the check is gone: the walk consumes an open directory descriptor and acts only relative to it — unlinkat(dfd, name, 0) first, which removes a file or a symlink and never follows one; only when the kernel answers EISDIR (Linux) or EPERM (BSD) is the entry opened with openat(dfd, name, O_RDONLY|O_DIRECTORY|O_NOFOLLOW) and descended into, then removed with unlinkat(AT_REMOVEDIR). Nothing is stat'ed and then trusted. The removal test now plants a symlink from inside the package to a sibling directory: the link goes, the sibling's file survives.
• Two fixture files created with fopen("wb") (0666 masked by umask) — the same class this repository fixed in bba834a and 8dca787, and the same fix: open(O_WRONLY|O_CREAT|O_TRUNC, 0600) + fdopen(), which is how write_eapp_named() in the same file already creates the package; fopen stays for the Windows branch, where the mode bits do not apply.

ctest 44/44, test_pkg_install_paths 10/10.

[srpatcha]

The path-traversal fix is solid: allowlisted names, install_path rebuilt and compared, symlink-safe removal. Its tests fail on master and pass here. One blocking issue: the component-name check allows a leading -, and eos_pkg_stop() now runs {"pkill", "-f", pkg->name}. A package named -u0 therefore becomes pkill -f -u0, which signals every root process (confirmed in WSL: pgrep -f -u0 lists PID 1, 2, 6 …). Fix: refuse a leading - in eos_pkg_field_is_component(), or pass -- before the name.

[Aswin-V]

Daily-loop code review (the loop does review; not a merge).

The pkg fix is sound and well-tested. eos_pkg_field_is_component() enforces the right invariant — every header field that becomes a path is a single terminated, non-empty [A-Za-z0-9._-] component, never ./.. — applied at all four trust boundaries (verify, install, update, load_db), so a poisoned db record from an earlier version is dropped rather than acted on. The fd-based recursive delete (O_NOFOLLOW, symlinks unlinked not followed) correctly replaces the system("rm -rf ...") shell escape, and the new 458-line test_pkg_install_paths.c covers the traversal vectors. This part answers #161.

Same bundling concern as #165: this PR also carries the unrelated kernel-scheduler rework (kernel/src/task.c, kernel/include/eos/kernel.h) and the linux_security.c NULL guard. Recommendation: keep this PR to the pkg fix + its tests, and split the scheduler and linux-security changes into their own PRs.

[Kartikey1306]

@Aswin-V done: this PR is now only the pkg fix and its tests.

The scheduler and linux_security commits were already on master as the identical commits, so rebasing onto master dropped them. Nothing needs splitting out.

Rebased head a369270 touches four files: services/pkg/eos_pkg.c, tests/test_pkg_install_paths.c, tests/CMakeLists.txt and CHANGELOG.md.

Two things had to change because of your v2 envelope (d1b52bb):

• Conflict in eos_pkg_verify(). Your free(res_data) after the signature check is kept, and the header-name check runs right after it. Every allocation is still freed on every path.
• One new commit, a369270. The test fixtures embedded the RFC 8032 Test 2 vector, which is a v1 signature over the binary alone, so 2 of the 10 tests failed on signature verification before reaching the name checks. They are now signed at test time over eos_pkg_envelope_digest() with the same key test_pkg_trust_anchor.c uses.

Measured at a369270 on macOS arm64 (-DEOS_BUILD_TESTS=ON, MacOSX26.5 SDK):

• ctest: 48/48 passed, and test_pkg_install_paths passes 10/10.
• The test discriminates. With master's eos_pkg.c swapped in, it fails: [FAIL] test_pkg_install_paths.c:230: eos_pkg_install(&db, EAPP_PATH) != 0. A validly signed package whose package_id leaves apps_dir installs.

Could you take another look?

[srpatcha]

Pushed 87ed567 to address my blocking review: eos_pkg_field_is_component() refuses a leading -, and pkill gets -- before the name. New test_install_refuses_a_leading_dash (-u0, -9, --help, - as name and package_id; com.example.my-app still installs) fails on a369270 at line 282 and passes after. WSL: pkg ctest 3/3.