system/nxpkg: Add network sync and harden package lifecycle.#3642
Draft
aviralgarg05 wants to merge 5 commits into
Draft
system/nxpkg: Add network sync and harden package lifecycle.#3642aviralgarg05 wants to merge 5 commits into
aviralgarg05 wants to merge 5 commits into
Conversation
This was referenced Jul 16, 2026
aviralgarg05
force-pushed
the
gsoc/nxpkg-sync-lifecycle-hardening-pr5
branch
2 times, most recently
from
July 17, 2026 19:47
d33ae3b to
c4f639c
Compare
acassis
reviewed
Jul 21, 2026
linguini1
requested changes
Jul 21, 2026
halyssonJr
reviewed
Jul 23, 2026
halyssonJr
reviewed
Jul 23, 2026
halyssonJr
reviewed
Jul 23, 2026
halyssonJr
reviewed
Jul 23, 2026
halyssonJr
reviewed
Jul 23, 2026
Fill in most of what the initial nxpkg slice deferred: network sync and artifact acquisition over plain HTTP (verified via SHA-256), an install rewrite that supports network sources and reclaims state left by an interrupted previous install, and wiring update/remove/rollback/ available into the CLI. Harden the package path against untrusted input along the way: bounded memory-safety helpers and explicit size limits throughout, storage read/write hardened against partially-written files, path-traversal rejection in manifest name/version before they reach the filesystem, and a fix for a lost-update race on the shared installed-packages database (two concurrent installs of different packages could otherwise silently clobber each other's recorded state). Add an optional manifest icon field for the nxstore GUI frontend to consume, raise PKG_INDEX_MAX now that the in-memory index is heap-allocated, and document the repository layout and local server setup in system/nxpkg/README.txt. Also fixes a real build break: pkg_runtime_compat() unconditionally referenced CONFIG_ARCH_BOARD, a Kconfig string symbol with no default clause under ARCH_BOARD_CUSTOM, so it is left entirely undefined rather than defined-but-empty on a custom board - falls back to CONFIG_ARCH_BOARD_CUSTOM_NAME instead. Routes pkg_error()/pkg_info() through syslog rather than stdio, since neither is visible to a supervisor with no attached console. Assisted-by: Claude:claude-sonnet-5 Signed-off-by: aviralgarg05 <gargaviral99@gmail.com>
Fix real correctness/security gaps found during review, on top of the network sync and CLI completion work: - pkg_install(): commit the installed database before writing the current/previous pointer files, and before advancing transaction state past ACTIVATED. Those are recovery bookkeeping and convenience mirrors respectively - once the installed database itself is durably committed, a failure to refresh either one must not trigger the failure-cleanup path, which could otherwise delete a payload the database now legitimately points at. Also stop treating an existing version directory as newly created when reinstalling an already-installed version, so a failed reinstall can't delete a working install. - pkg_uninstall()/pkg_rollback(): acquire the per-package install lock in addition to the installed-db lock, drop the entry from the authoritative database first, and only then remove payload files - a crash between those two steps can leave orphaned files, which are reclaimable, but never leaves the database pointing at a payload that's already gone. - pkg_sync(): stage each sync to a PID-qualified temp filename instead of a single fixed name, so concurrent sync calls can no longer race on the same staging file. Return real negative errno codes instead of EXIT_SUCCESS/EXIT_FAILURE, matching the rest of this API; pkg_main.c maps that back to a process exit code at the CLI boundary. - pkg_metadata_parse_installed_entry(): validate that name/current/ previous/every entry in versions[] are safe path components, and that current (and previous, if set) actually appear in versions[] - a corrupted or tampered installed-packages database can no longer reference a nonexistent version or smuggle a path-traversal sequence through a field this code already trusted implicitly. - pkg_store_write_all()/pkg_repo_sink(): treat a zero-byte write() as -EIO instead of looping on it silently. - Removed the malloc()-falls-back-to-kmm_malloc() logic in pkg_malloc/ pkg_zalloc/pkg_realloc/pkg_free entirely - it required tracking which allocator owned a given pointer via kmm_heapmember(), which is specific to this target's flat, single-heap memory model and not a sound general application API. These are now plain wrappers around malloc/calloc/realloc/free. - Added pkg_metadata_load_manifest_path(), so a caller (e.g. the nxstore GUI frontend, launching an installed package) can load the manifest actually recorded for a specific installed version, instead of combining a rollback-selected version with whatever the current catalog happens to describe for that package name - those can disagree after a rollback if the catalog has since moved on. - Storage root default changed from /tmp/nxpkg to /var/lib/nxpkg, following the conventional persistent application-data location instead of a path whose own name suggests non-persistent storage. A board without persistent storage mounted at /var, or that wants a different location (e.g. an SD card), still overrides this via CONFIG_SYSTEM_NXPKG_ROOT as before. - Moved system/nxpkg/README.txt into the companion NuttX documentation PR rather than shipping user-facing documentation as an in-tree README. Assisted-by: Claude:claude-sonnet-5 Signed-off-by: aviralgarg05 <gargaviral99@gmail.com>
- pkg_sync(): index.jsn and repo.url were updated as two independent atomic writes with nothing serializing the pair against a second, concurrent pkg_sync() call - each write stayed internally consistent, but the pair didn't, so one sync's index could end up on disk next to a different sync's source URL. Add a dedicated sync lock (pkg_repo_acquire_sync_lock(), mirroring the existing installed- db lock's blocking-retry-with-stale-reclaim pattern) around the whole read-fetch-write sequence. Also renew the lock's mtime as data actually arrives (pkg_repo_sink(), via a new renew_lock_path field threaded through pkg_acquire_source()) rather than only stamping it once at acquire time - a lock acquired once and then measured against a fixed 10-minute staleness window could otherwise be reclaimed mid-download on a large-enough file over a slow-enough link, even though the download was still genuinely in progress. pkg_reclaim_stale_lock() (renamed from pkg_install_reclaim_stale_lock, made public) is shared between both lock kinds rather than duplicated. - pkg_install_prune_oldest_version(): deleted the pruned version's on-disk payload directory before the updated installed database was even durably saved. If pkg_metadata_save_installed() subsequently failed, the payload was already gone but the last successfully-saved instpkg.jsn could still list that version as installed. The victim version is now handed back to the caller (threaded through pkg_install_add_version()/pkg_install_update_installed()) so pkg_install() can defer the actual directory removal until after the save succeeds. - pkg_metadata_version_token_cmp(): two version tokens with equal numeric prefixes (e.g. "1a" and "1b", both parsing as 1) compared as equal instead of falling back to a lexical comparison of what follows the number, contradicting this function's own documented behavior and silently treating genuinely different versions as the same one. Compare the non-numeric remainder lexically when the numeric prefixes match instead of falling through. Assisted-by: Claude:claude-sonnet-5 Signed-off-by: aviralgarg05 <gargaviral99@gmail.com>
Shorten the installed-database lock description and centralize pkg_sync() cleanup, as requested in review. Replace repeated protocol literals with named constants and preserve errno before cleanup calls can overwrite it. Store the synchronized catalog source in the catalog itself so the catalog and its relative-artifact base are committed atomically. Continue reading the former sidecar format for upgrade compatibility, and normalize array-form catalogs before adding the private source field. Compare numeric version prefixes without strtol() overflow and retain lexical comparison of suffixes. Assisted-by: Codex:gpt-5 Signed-off-by: aviralgarg05 <gargaviral99@gmail.com>
Record a per-boot token and owner PID in every package, installed-database, and synchronization lock. A contender now keeps a lock held while its recorded owner task is alive, regardless of unreliable FAT modification timestamps. Reclaim locks from exited owners or earlier boots immediately, while retaining timestamp-based migration handling for legacy empty lock files. This prevents a just-created live lock from being mistaken for a decades-old stale file on targets whose mounted filesystem clock does not match CLOCK_REALTIME. Assisted-by: OpenAI Codex:gpt-5.6-sol Signed-off-by: aviralgarg05 <gargaviral99@gmail.com>
aviralgarg05
force-pushed
the
gsoc/nxpkg-sync-lifecycle-hardening-pr5
branch
from
July 24, 2026 10:01
c4f639c to
f363a8f
Compare
This was referenced Jul 24, 2026
This was referenced Jul 24, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Note: Please adhere to Contributing Guidelines.
Summary
Complete the network and package-lifecycle work started by #3474.
This adds:
netutils/webclient;sync,available,update,remove, androllbackcommands;that do not depend on filesystem timestamp accuracy;
version fields;
frontends; and
CONFIG_ARCH_BOARDis not defined.The synchronized catalog records its source in the same atomically replaced
file. This keeps relative artifact resolution consistent with the catalog
under concurrent synchronization. The previous
repo.urlsidecar is stillread for migration from development images.
User documentation is in apache/nuttx#18875. #3643 is the graphical frontend
that depends on the APIs added here.
Impact
nxpkggains repository synchronization and theinstall/update/remove/rollback lifecycle described above.
/var/lib/nxpkg; boards must mount persistent storage there or overrideCONFIG_SYSTEM_NXPKG_ROOT.they use the selected
netutils/webclientHTTP/TLS facilities.to the downloaded catalog, but is not package signing. Plain HTTP does not
authenticate the repository.
CONFIG_SYSTEM_NXPKGdisabledare unaffected. The legacy
repo.urlsidecar remains readable formigration.
rewritten.
Testing
Build host:
xtensa-esp-elf-gcc 14.2.0(
esp-14.2.0_20251107)Target:
Verification:
nxstyleon all changed C sourcesgit diff --checksystem/nxpkgsources cross-compiled for ESP32-S3system/nxpkgsources compiled in the simulator configurationpersistence, rollback, remove, and relative-artifact execution
committed exactly one install
-EBUSYtransaction file
Testing logs before change:
Testing logs after change:
Final hardware evidence to attach before publication:
run.
PR verification Self-Check