diff --git a/local/docs/DBUS-INTEGRATION-PLAN.md b/local/docs/DBUS-INTEGRATION-PLAN.md index 23c4212db7..0ee3da7208 100644 --- a/local/docs/DBUS-INTEGRATION-PLAN.md +++ b/local/docs/DBUS-INTEGRATION-PLAN.md @@ -1,8 +1,8 @@ # Red Bear OS D-Bus Integration Plan **Implementation status (2026-07-26):** All DBUS plan code artifacts are build-verified. Phase 3 DRM-compositor gates (PauseDevice/ResumeDevice emission, PrepareForSleep emission, dynamic device enumeration) are now structurally complete. **Phase 4 re-enablement progress:** 20/24 KF6 frameworks have USE_DBUS=ON. **redbear-polkit v0.2** now implements real authorization (subject UID extraction, `*` / `@group` / `!uid` / `!@group` policy syntax, default-deny for unknown actions). **redbear-udisks v0.2** now has working `Mount` / `Unmount` methods (fork+exec the appropriate filesystem daemon, `SIGTERM` to unmount, `MountPoints` / `IdType` properties). **redbear-notifications v0.2** now emits the `ActionInvoked` signal via the new `InvokeAction` method. **redbear-upower v0.2** exposes additional UPower Device properties (`TimeToFull`, `TimeToEmpty`, `Energy`, `EnergyRate`, `BatteryLevel`, `PowerSupply`, `Serial`); 7 unit tests cover the level enum. **redbear-statusnotifierwatcher v0.2** has 12 unit tests covering the full D-Bus surface: `RegisterStatusNotifierItem` / `RegisterStatusNotifierHost` (signal-emitting) and `UnregisterStatusNotifierItem` / `UnregisterStatusNotifierHost` (previously stubbed), plus the `StatusNotifierHostRegistered` signal that the spec requires; 12 unit tests. **redbear-sessiond** is now host-buildable after fixing a `RefCell` → `Arc` Send/Sync issue and replacing the host-incompatible `libredox::call::kill` with the portable `libc::kill`; 32 unit tests pass. **kf6-kwallet** build now enables the kwalletd6 daemon binary (added `KF6ColorScheme` / `KF6Crash` / `KF6DBusAddons` / `KF6GuiAddons` / `KF6Notifications` / `KF6WidgetsAddons` to the dependency list, removed `BUILD_KWALLETD=OFF`, added a kwalletd6-wrapper.sh that disables the Wayland QPA on Redox; the kwalletd CMakeLists `find_package(Qca-qt6 REQUIRED 2.3.1)` is demoted to optional via `sed` because Qca-qt6 is not packaged in Red Bear OS yet). **polkit-qt6-1 (PolkitQt6-1)** is now packaged from the upstream 0.200.0 tarball, giving kf6-kauth a real Qt6 binding for the org.freedesktop.PolicyKit1 D-Bus API. **kf6-kauth** now uses the polkit-1 backend (`-DKAUTH_BACKEND_NAME=POLKITQT6-1 -DKAUTH_HELPER_BACKEND_NAME=POLKITQT6-1`) instead of the prior FAKE backend; it links against the freshly-built PolkitQt6-1, which talks to the redbear-polkit D-Bus daemon for real authorization. **kf6-kded6** now builds the kded6 daemon binary (v4.0); the recipe wraps it with offscreen QPA. **kf6-kglobalaccel** now also builds the kglobalacceld5 daemon binary (v4.0); the recipe wraps it with offscreen QPA. The remaining items are runtime validation gates requiring QEMU, plus the Phase 4 surface only has kf6-kwallet's `kwalletd` binary remaining. -**Version:** 4.0 — 2026-07-26 -**Status:** Active plan aligned with desktop path v6.0 (2026-07-26) +**Version:** 4.1 — 2026-07-27 +**Status:** Active plan aligned with desktop path v6.0 (2026-07-26); §16 round-2 review appended 2026-07-27 **Scope:** Full D-Bus infrastructure for KDE Plasma 6 on Wayland, tightly integrated with Redox scheme IPC **Parent plan:** `local/docs/CONSOLE-TO-KDE-DESKTOP-PLAN.md` (v6.0, 2026-07-26) @@ -122,7 +122,7 @@ specific schemes it needs. This keeps the architecture honest and avoids a leaky | **redbear-polkit policy file** | `config/redbear-full.toml`, `config/redbear-mini.toml` | ✅ done (v3.2) | `/etc/polkit-1/policy.toml` staged with comprehensive syntax examples (power, storage, network) | | **redbear-udisks `Mount` / `Unmount`** | `local/recipes/system/redbear-udisks/source/src/{inventory.rs,interfaces.rs,mount.rs}` | ✅ done (v3.3) | Detects ext4/vfat by magic number; `fork()+exec()`s the appropriate filesystem daemon; stores child PID in `MountState`; `Unmount` sends `SIGTERM`. 9 unit tests cover the detection paths. | | **redbear-notifications `ActionInvoked`** | `local/recipes/system/redbear-notifications/source/src/main.rs` | ✅ done (v3.3) | `InvokeAction(id, action_key)` method emits the `ActionInvoked` signal. ServerVersion bumped to 0.2.0. | -| **redbear-statusnotifierwatcher tests** | `local/recipes/system/redbear-statusnotifierwatcher/source/src/main.rs` | ✅ done (v3.3) | 12 unit tests cover item/host registration (signal-emitting), unregistration, idempotency, and snapshot correctness. Helper methods extracted to be testable. *(Test count verified 2026-07-27: `grep -c '#\[test\]' main.rs` = 12.)* | +| **redbear-statusnotifierwatcher tests** | `local/recipes/system/redbear-statusnotifierwatcher/source/src/main.rs` | ✅ done (v3.3, expanded round 2) | 22 unit tests cover item/host registration (signal-emitting), unregistration (owner-validated), idempotency, snapshot correctness, cross-owner enforcement, NameOwnerChanged purge, input validation, and bounded eviction. *(Test count updated 2026-07-27 round 2: see §16 G1.)* | ### 3.2 What Exists But Is Incomplete @@ -1369,3 +1369,285 @@ for D-Bus runtime validation — the session-bus and activation gaps (F6) remain | zbus marker recipe | Yes (no-op) | N/A (Cargo resolves real crate) | F2 above | | dbus-send / dbus-launch built | Yes | No (not runtime-tested) | `tools/meson.build` | | Kernel/relibc epoll fixes | Yes (in fork history) | No (not exercised under real bus load) | F7 above | + +--- + +## 16. Round 2 Findings (2026-07-27) + +This section records verified evidence from a second read-only review of the +D-Bus integration surface, conducted as the doc-cleanup phase of the second +implementation round (the 5-lane review). It follows the same evidence +discipline as §15: every finding cites an exact path, distinguishes +**working-tree present** (the code exists in the current source tree, including +uncommitted round-2 changes) from **runtime-proven** (validated in QEMU or +bare metal), and labels uncertainty where evidence is inconclusive. + +Cross-reference: §15 (2026-07-27 round-1 review) established the build-wired +vs runtime-proven baseline. This section documents the round-2 delta against +that baseline. + +### G1. StatusNotifierWatcher — sender validation, owner-keyed registry, and lifecycle purging (APPLIED) + +**Path:** `local/recipes/system/redbear-statusnotifierwatcher/source/src/main.rs` + +The StatusNotifierWatcher was substantially rewritten in round 2. The changes +verified in the working tree: + +- **Sender-validated registration.** `register_status_notifier_item`, + `register_status_notifier_host`, `unregister_status_notifier_item`, and + `unregister_status_notifier_host` now accept `#[zbus(header)] hdr: Header<'_>` + and extract the caller's unique bus name via `hdr.sender()`. Registrations + are stored in an owner-keyed `Registry` (`HashMap>`) + rather than a flat `HashSet`. Only the owning caller may unregister its own + entries — a wrong-owner unregister returns `false` without side effect. + +- **Input validation.** `validate_input()` rejects empty strings, strings + longer than `MAX_INPUT_LEN` (256), and strings containing NUL or other + control characters (`char::is_control()`). Applied to both item and host + registration parameters. + +- **NameOwnerChanged purging.** A background `tokio::spawn` task subscribes to + `org.freedesktop.DBus.NameOwnerChanged` via a `#[proxy]` trait. When a bus + name vanishes (`new_owner` empty, `old_owner` non-empty), + `watcher.purge_owner(&old_owner)` removes all items and hosts owned by that + name. This prevents stale entries from disconnected clients. + +- **Bounded entries.** `MAX_ENTRIES = 1024` with FIFO eviction + (`evict_oldest()` drops the front of `insertion_order`). Prevents unbounded + growth from a misbehaving or hostile client. + +- **Test coverage.** 22 unit tests (up from 12): ownership enforcement (wrong + owner cannot unregister), purge on owner loss, input validation (empty, + oversized, control chars), and bounded eviction (oldest evicted at capacity, + empty-owner cleanup). All tests pass on the host. + +**Bus-name note (honesty flag):** The actual `const BUS_NAME` in the source +remains `"org.freedesktop.StatusNotifierWatcher"` (line 14, unchanged). The +`recipe.toml` header comment was updated from `org.freedesktop` to +`org.kde.StatusNotifierWatcher`, and the `#[interface(name = ...)]` annotation +remains `"org.freedesktop.StatusNotifierWatcher"`. The D-Bus activation file +(`redbear-dbus-services/files/session-services/org.freedesktop.StatusNotifierWatcher.service`) +and the `Name=` line inside it also use the `org.freedesktop` form. **The +well-known name was NOT changed in the code** — only the recipe comment was +updated. If the intended fix was to switch to `org.kde.StatusNotifierWatcher` +(the name Plasma's own `statusnotifierwatcher.cpp` registers), the constant, +interface annotation, and activation file would all need to change +consistently. As of the working tree at HEAD, they do not. + +### G2. redbear-notifications — sender-validated InvokeAction (APPLIED) + +**Path:** `local/recipes/system/redbear-notifications/source/src/main.rs` + +- **Notification ownership record.** A `NotificationRecord { owner: String, + action_keys: Vec }` is stored in a `Mutex>` on every `Notify()` call. The caller's unique bus name + (`hdr.sender()`) is captured as the owner, and declared action keys (even + indices of the `actions` array per the freedesktop spec) are extracted. + +- **InvokeAction sender validation.** `InvokeAction` now takes + `#[zbus(header)] hdr: Header<'_>` and returns `fdo::Result<()>` instead of + `()`. The `validate_invoke(id, caller, action_key)` helper checks three + conditions: (a) the notification ID is known, (b) the caller's unique bus + name matches the stored owner, and (c) the action key was declared in the + original `Notify` call. A spoofed `InvokeAction` from a different client is + rejected with `fdo::Error::Failed`. + +- **Lifecycle cleanup.** `CloseNotification` calls `remove_notification(id)` + so the ownership record is cleaned up. + +- **Test coverage.** New tests verify: correct owner + declared key → `Ok`, + wrong owner → error ("not notification owner"), unknown ID → error, and + post-removal invocation → error ("unknown notification id"). + +### G3. redbear-wifictl — NM root State enum 0..70 and OwnedObjectPath types (APPLIED) + +**Path:** `local/recipes/system/redbear-wifictl/source/src/dbus_nm.rs` + +- **NmState enum (0..70).** A new `NmState` enum (`Unknown=0, Asleep=10, + Disconnected=20, Disconnecting=30, Connecting=40, ConnectedLocal=50, + ConnectedSite=60, ConnectedGlobal=70`) was added. This is distinct from + `NmDeviceState` (0..120). The root `org.freedesktop.NetworkManager.State` + property now returns `NmState::from_device_state(...).as_u32()` instead of + the raw per-device state. This closes a class-math bug where a + `NmDeviceState` value (e.g. `Activated=100`) could leak onto the root + `State` property, exceeding the 0..70 `NMState` contract that Qt's + `QNetworkConfigurationManager` expects. + +- **OwnedObjectPath return types.** `get_devices()`, `get_all_devices()`, + `active_connections()`, `access_point_paths()`, `active_access_point()`, and + `device_path()` all changed from returning `String`/`Vec` to + `OwnedObjectPath`/`Vec`/`fdo::Result`. A + `try_path()` helper converts path strings to `OwnedObjectPath` with + `fdo::Error` mapping instead of `.expect()`. This matches the typed-path + convention already used by `redbear-sessiond`, `redbear-udisks`, and + `redbear-upower`. + +- **Test coverage.** New tests verify: `NmState` default is `Unknown` (0), + `as_u32()` matches the NetworkManager spec for all 8 variants, + `from_device_state` maps `Activated → ConnectedGlobal`, connecting states → + `Connecting`, and disconnected states → `Disconnected`. A dedicated test + asserts that `NmState` never exceeds 70 for any `NmDeviceState` input. + +### G4. redbear-sessiond — test environment guard (APPLIED) + +**Path:** `local/recipes/system/redbear-sessiond/source/src/manager.rs` (test module) + +The `can_methods_return_na` test previously hard-coded `assert_eq!(... +"yes")` for `can_power_off`/`can_reboot`/`can_suspend`. On a Linux host +(where `cargo test` runs for host-runnable unit tests), the `kstop_writable()` +probe returns `false` (no `/scheme/sys/kstop`), so the `Can*` methods +correctly return `"na"` — causing the test to fail. The fix mirrors the +runtime detection: + +```rust +let expected = if kstop_writable() { "yes" } else { "na" }; +assert_eq!(manager.can_power_off().unwrap(), expected); +``` + +This allows the test to pass on both Redox (where kstop is writable → "yes") +and Linux host (where kstop is absent → "na"). No production code changed; +only the test assertion adapts. + +### G5. Honest capabilities — pre-existing (CONFIRMED) + +**Path:** `local/recipes/system/redbear-notifications/source/src/main.rs` + +`GetCapabilities()` returns `vec!["body".to_owned(), "body-markup".to_owned()]` +— an honest minimal set. A unit test explicitly asserts that `"actions"` and +`"persistence"` are NOT advertised until the corresponding features are +implemented. This was already present at HEAD (round-1 or earlier); round 2 +did not change it. + +### G6. verify-fork-functions exclusion — pre-existing, narrowly scoped (CONFIRMED) + +**Path:** `local/scripts/verify-fork-functions.sh` + `.verify-fork-functions.exclude` files + +The script only covers `local/sources/*/` forks with upstream remotes. D-Bus +daemons live in `local/recipes/system/redbear-*/source/` and are **not in +scope** for fork-function verification (they have no upstream). Three exclude +files exist (`base`, `kernel`, `installer`), each listing specific +`file:function_name` pairs with justifying comments. Zero blanket or D-Bus- +related exclusions. This was already present at HEAD; round 2 did not change it. + +### G7. write_all error propagation — pre-existing, partial (CONFIRMED) + +**Path:** `local/recipes/system/redbear-sessiond/source/src/manager.rs` + +`redbear-sessiond`'s power-off/suspend/reboot paths use +`if let Err(e) = f.write_all(b"shutdown")` with error recovery (resets +`preparing_for_shutdown` flag, returns `fdo::Error::Failed`). This was already +present at HEAD. Other daemons (`redbear-authd`, `redbear-udisks` test code, +`cpufreqd`) still swallow `write_all` errors with `let _ =`, `.unwrap()`, or +`.is_ok()`. **Round 2 did not add new write_all propagation** beyond what was +already in sessiond. The gap remains open for authd/udisks/cpufreqd. + +### G8. Inhibitor lifecycle reaping — NOT applied (GAP REMAINS) + +**Path:** `local/recipes/system/redbear-sessiond/source/src/manager.rs` + +The `inhibit()` method registers inhibitors: it creates a `UnixStream::pair()`, +stores the caller FD in `inhibitor_fds`, pushes an `InhibitorEntry` into +`runtime.inhibitors`, and returns the other end as `OwnedFd`. However, **no +reaping logic exists**: the `inhibitor_fds` and `runtime.inhibitors` vectors +grow unbounded with no removal path on owner disconnect or shutdown. + +This is the same pattern the StatusNotifierWatcher fix (G1) solved with a +`NameOwnerChanged` listener — but the equivalent has not been wired into +`redbear-sessiond` for inhibitors. Round 2 did not modify `manager.rs` +inhibitor code (only the test module changed, per G4). **This remains a gap.** + +### G9. Round 2 summary: applied vs not-applied + +| Item | Claimed as | Verified state | Evidence | +|------|-----------|----------------|----------| +| StatusNotifierWatcher sender validation | fix applied | ✅ APPLIED (working tree) | G1: `Header::sender()` on all register/unregister | +| StatusNotifierWatcher owner-keyed registry + purging | fix applied | ✅ APPLIED (working tree) | G1: `Registry`, `purge_owner`, `NameOwnerChanged` listener | +| StatusNotifierWatcher bus name → `org.kde` | fix applied | ⚠️ NOT APPLIED in code | G1: BUS_NAME still `org.freedesktop`; only recipe comment changed | +| NM root State enum 0..70 | fix applied | ✅ APPLIED (working tree) | G3: `NmState` enum, `from_device_state` | +| OwnedObjectPath types | fix applied | ✅ APPLIED (working tree) | G3: NM interface return types changed | +| Sender validation on invoke actions | fix applied | ✅ APPLIED (working tree) | G2: `validate_invoke` in notifications | +| Inhibitor lifecycle reaping | fix applied | ❌ NOT APPLIED | G8: no reaping code added to sessiond | +| write_all error propagation | fix applied | ⚠️ PARTIAL (pre-existing) | G7: sessiond has it; authd/udisks/cpufreqd do not | +| Honest capabilities | fix applied | ✅ PRE-EXISTING (unchanged) | G5: already present at HEAD | +| sessiond test env guard | fix applied | ✅ APPLIED (working tree) | G4: `kstop_writable()` guard in test | +| verify-fork-functions exclusion narrowed | fix applied | ✅ PRE-EXISTING (unchanged) | G6: already narrow at HEAD | + +### G10. Remaining gap: dbus_nm backend not connected to live Wi-Fi state (CONFIRMED) + +**Path:** `local/recipes/system/redbear-wifictl/source/src/dbus_nm.rs` + +The NM D-Bus interface is a static facade. Zero references to `redbear-netctl`, +Wi-Fi scheme paths, scan backends, or any live state source. The +`register_nm_interface()` function creates a hardcoded device +(`NmWifiDevice::new_disconnected("wlan0")`). `request_scan()` returns +`Ok(())` without performing a scan. The `NmWifiDevice` struct is purely +in-memory. The `dbus-nm` feature is **not** in the default feature set +(`default = []`), so the entire module is compiled out in default builds. + +This is consistent with the plan's deferral of NetworkManager (Red Bear OS +uses `redbear-netctl`), but the facade's existence should not be mistaken for +a working NM backend. **No change in round 2.** + +### G11. Remaining gap: hardcoded root password in redbear-full.toml (CONFIRMED — operator decision, out of scope) + +**Path:** `config/redbear-full.toml` (lines 102–106) + +```toml +[users.root] +password = "password" +``` + +The root password is the **plaintext** string `"password"` — not a salted hash +(`$6$...`). This is a development-time convenience. Replacing it with a proper +SHA-crypt hash (or removing the password line to force first-boot setup) is an +**operator decision**, tracked separately from the D-Bus integration surface. +**No change in round 2; explicitly out of scope per operator.** + +### G12. Remaining gap: NM root StateChanged signal not yet emitted (CONFIRMED) + +**Path:** `local/recipes/system/redbear-wifictl/source/src/dbus_nm.rs` + +No `StateChanged` signal is defined or emitted on the NM root or device +interface. The `NmRoot` interface defines properties (`state`, +`wireless_enabled`, etc.) but no `#[zbus(signal)]` declaration. No +`PropertiesChanged` emission exists either. A comment at line 122 references +`StateChanged` in the `NmState` docstring, but no signal code follows. + +Clients that poll `State` will see correct values (thanks to G3's +`NmState::from_device_state` mapping), but clients that subscribe to +`StateChanged` or `PropertiesChanged` for change notification will not receive +updates. **No change in round 2.** + +### G13. Remaining gap: Mesa batch pool class math fix (SEPARATE MESA PATCH — not in this round) + +**Path:** `local/patches/mesa/` (10 patches) + +No patch matching "batch pool" or "class math" exists in `local/patches/mesa/`. +The closest patch is `26-cs-submit-bidirectional-seqno.patch` (batch BO +submission for compute-shader ring seqno synchronization), which is unrelated +to a class/arithmetic fix. This item is tracked as **separate Mesa work** +outside the D-Bus integration round. The Mesa redox gallium winsys BO byte +count was fixed in an earlier round (per `SUPERSEDED-DOC-LOG.md` Round 9 +post-doc, commit `aae6c36b80`), but the specific "batch pool class math" issue +referenced by the 5-lane review is either not yet landed or lives in a +different patch series. + +### G14. Round 2 verification methodology + +All findings in this section were verified by reading the actual source files +in the working tree (including uncommitted round-2 changes, confirmed via +`git diff`), not just the committed HEAD. Five files show uncommitted +modifications relevant to this round: + +| File | Change summary | +|------|---------------| +| `redbear-statusnotifierwatcher/source/src/main.rs` | Full rewrite: owner-keyed registry, sender validation, NameOwnerChanged purging, input validation, bounded entries | +| `redbear-statusnotifierwatcher/recipe.toml` | Comment updated: `org.freedesktop` → `org.kde` | +| `redbear-notifications/source/src/main.rs` | NotificationRecord ownership, validate_invoke sender check | +| `redbear-sessiond/source/src/manager.rs` | Test env guard (kstop_writable) in can_methods_return_na | +| `redbear-wifictl/source/src/dbus_nm.rs` | NmState enum 0..70, OwnedObjectPath return types | + +Host unit tests for all modified daemons pass (`cargo test` without +`--target`, the sanctioned test path for pure-logic crates). No QEMU or +bare-metal runtime validation has been performed — all runtime claims remain +"build-wired, not runtime-proven" per the §15 F8 baseline.