Skip to content

store: the app store capsule - #545

Open
eKisNonos wants to merge 4 commits into
mainfrom
store/app-store
Open

eKisNonos wants to merge 4 commits into
mainfrom
store/app-store

Conversation

@eKisNonos

Copy link
Copy Markdown
Contributor

New capsule. Reads the signed marketplace index, lists what the running image can actually install, and installs through the queue init already owns.

The install buttons did not work. The painter and the hit test each worked out their own rectangles, so a click never matched the row it looked like it hit and nothing fired. Both come from one geometry module now and the click path lands on the same install::ask that Enter does.

Search filters as you type, the list has a real scrollbar off the same geometry, and selection survives an index refresh instead of jumping to the top.

Two bits outside the capsule: init gains an install queue and a wake path so a request is handled when it arrives rather than on the next supervisor spin, and the surface registry can attach frames to a surface a capsule owns, which is how the store draws its own window.

Scrollbar arithmetic mirrored in python: empty list, list shorter than the viewport, thumb at both ends, and the single row overflow where an off by one would not be visible.

Reads the signed marketplace index, lists what the running image can
install, and installs through the queue init already owns.

The install buttons did nothing: the painter and the hit test each
computed their own rectangles, so a click never matched a row. Both
derive from one geometry module now and the click path reaches the
same install::ask as Enter.

Search filters as typed, the list scrolls with a real scrollbar, and
selection survives a refresh.

init gains an install queue and a wake path so a store request is
serviced on arrival rather than on the next supervisor spin. The
surface registry can attach frames to a surface a capsule owns.

Scrollbar arithmetic mirrored in python: empty, shorter than the
viewport, thumb at both ends, single row overflow.
text::line returns the drawn width, so the bare match evaluated to i32
where the function body expects (), failing the capsule build.
@senseix21

Copy link
Copy Markdown
Collaborator

@eKisNonos Review before merge

Scope: new capsule_app_store (Marketplace) + kernel mirror src/userspace/capsule_app_store, init install queue/wake, dock entry (LAUNCHER_APPS 12→13, LauncherIcon::Store), assets/icons/store.{svg,a8}.

Our changes: 8a0dd05ed merged origin/main (no conflicts left; hand-synced lists consistent). 1d95cf3f6 fixed store/ui/searchbar.rs:47 — bare match evaluated to i32 (E0308); now a statement.

Findings (blocking):

Verdict: hold — depends on unmerged work (#535, the #546 foreign/linux-install surface, #544). Rebase-free fix: land those first, then merge main here.

@senseix21

Copy link
Copy Markdown
Collaborator

Reviewed at 1d95cf3f6 (not a draft, base main, 76 files, +3220/−101, 213 commits behind main, 4 commits of its own, last pushed 2026-09-24). A new userland/capsule_app_store at +2662, tools/nonos-icon-store +108, and 442 added lines across src/userspace and src/kernel_core.

Verdict: Request changes. The branch does not compile, and the cause is not in the store code — two merge commits dropped definitions while keeping their call sites. One of the two casualties is a security guard, so the tempting quick fix is the wrong one.

Critical

1. build-x86_64-capsules fails on two missing symbols, both merge damage.

From ci-reports-build/build/build-x86_64.txt:

error[E0425]: cannot find function `is_foreign` in module `crate::process::foreign`
error[E0425]: cannot find function `spawn_install` in module `crate::userspace::capsule_linux`

Verified at this head rather than taken from the log:

Meanwhile the caller of the first one is present: src/kernel_core/surface_registry/share/attach_surface.rs:31-33 is

// Never to a guest.
if crate::process::foreign::is_foreign(receiver_pid) {
    return Err(RegistryError::InvalidArg);
}

So this branch's merges (5d1ec5484 and 8a0dd05ed, both "Merge remote-tracking branch 'origin/main'") took the call site from main and not the definition it needs. That is a conflict resolved the wrong way, and it is why nothing downstream of the build lane — boot-smoke, benchmark, attestation, every dark-features-compile variant — has anything to say about this PR.

The fix direction matters here. Deleting the is_foreign call would make it compile and would silently remove the check that stops a surface being attached to a Linux guest. Restore the definition and the re-export instead: is_foreign in src/process/foreign/registry.rs and its name in the mod.rs pub use. Same for spawn_install — find what it was and restore it, rather than removing whatever calls it.

Given the 213-commit gap, redoing the merge against current main is probably less work than patching these two by hand, and it is the only way to find out whether there are more casualties that the first compile error masked. rustc stops reporting after the ones it has; two named symbols is a floor, not a count.

Important

2. Every other red lane is downstream of item 1 or is nine days stale.

The build report (run 36025260977, 2026-09-24) is:

pass  rustfmt              pass  clippy-nonos-sign
fail  build-x86_64-capsules     pass  section-size
pass  symbol-scan          gap   build-aarch64

Note what is not in that list: no tcb-budget, no proof-coverage. Those appear in every current build report in this queue, so this run is from an older nonos-verify against an older main. And because pull_request CI builds refs/pull/N/merge, these verdicts describe this head merged into a main that no longer exists.

So there are really two separate problems: the branch does not compile (item 1, real, in the diff), and nothing else the lanes say can be trusted until it is rebased and re-run.

Verified correct

The store code itself reads well, and the kernel-side additions are the parts I checked most closely because they are the ones that could matter beyond the capsule.

  • self_attach is correctly guarded and correctly placed. The new src/kernel_core/surface_registry/share/self_attach.rs is a fast path for the owner attaching its own surface — it returns the owner's existing VA rather than making a second mapping. It validates the handle epoch against the slot (slot.epoch != epoch → BadHandle), which is what stops a stale handle naming a recycled slot, and it refuses to be a fast path for anyone else: if slot.owner_pid != receiver_pid || slot.owner_base_va == 0 { return Ok(None) }, returning None so the caller falls through to a real mapping rather than erroring.
  • It cannot be reached by a guest. In attach_surface, the is_foreign refusal is the first statement, ahead of the attach_map::lookup fast path and ahead of self_attach. So the new path inherits the existing guest guard rather than sidestepping it. I checked the order specifically, because a fast path inserted above a guard is how that kind of check gets lost.
  • It drops the registry lock before calling out. drop(slots) precedes descriptor(handle) and attach_map::record, so the SLOTS mutex is not held across another registry call. That is the same discipline the rest of this codebase relies on and it is easy to omit in a new file.
  • The spawn wiring is complete. src/userspace/capsule_app_store/{embed,mod,spawn,state}.rs plus the spawn_plan/apps.rs entry is the full path from embedded artifact to spawned capsule — a 2662-line capsule with no spawn plan would be dormant, and this is not that.
  • rustfmt, clippy-nonos-sign, section-size and symbol-scan all pass, so the failure really is the two unresolved names and not a broader problem with the new code.

Questions

  1. Was is_foreign perhaps renamed on main between this branch's two merges, rather than dropped? If so the fix is a call-site rename and not a restoration, and knowing which it is decides whether attach_surface's guard is currently wired to something that exists under another name.
  2. src/userspace/init/{install_queue,wake}.rs are new (+61 and +56) and init/supervisor/loop_impl.rs loses 40 lines. Is the supervisor loop now driven by the wake path rather than polling, and if so does the app store's install queue share that mechanism with capsule_installer's, or are there two queues?

CI at this head

Fifteen lanes pass. build-x86_64-capsules fails on item 1 and takes boot-smoke, benchmark, attestation and the dark-features-compile matrix with it. Everything else is nine days stale against a main that has moved 213 commits.

Item 1 is the whole of it: restore the two symbols — or redo the merge — and re-run before anything else here can be judged.

@senseix21

Copy link
Copy Markdown
Collaborator

Follow-up to item 1, with the fix located rather than guessed at.

#583 (23f3c5599) contains this PR, and both symbols that are undefined here exist there:

  • is_foreign — defined at src/process/foreign/registry.rs:40, exported at src/process/foreign/mod.rs:60 as pub use registry::{clear, is_foreign, supervisor_of};
  • spawn_install — src/userspace/capsule_linux/install.rs:36, pub fn spawn_install(package: &str, pinned: &[u8; 32]) -> Result<u32, SpawnError>

So this confirms the diagnosis: both were lost in this branch's two origin/main merges rather than never having existed, and spawn_install's signature above is what to restore. It also means attach_surface's "Never to a guest" guard should be made to compile, not removed.

Worth deciding whether this PR needs to land separately at all. #583 carries 50 of this branch's 52 capsule_app_store files plus three more, and it carries #544 too.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants