fix: keep wallet service alive and verify saved profiles
Some checks failed
Check / check (push) Has been cancelled
Some checks failed
Check / check (push) Has been cancelled
This commit is contained in:
2
.github/workflows/check.yml
vendored
2
.github/workflows/check.yml
vendored
@@ -17,7 +17,7 @@ jobs:
|
||||
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
|
||||
with:
|
||||
persist-credentials: false
|
||||
- run: sudo apt-get update && sudo apt-get install -y libsecret-1-dev libglib2.0-dev pkg-config dbus-daemon openssl
|
||||
- run: sudo apt-get update && sudo apt-get install -y libsecret-1-dev libglib2.0-dev pkg-config dbus dbus-daemon gnome-keyring openssl
|
||||
- run: rustup toolchain install stable --profile minimal --component rustfmt --component clippy && rustup default stable
|
||||
- run: cargo fmt --check
|
||||
- run: cargo clippy --all-targets --locked -- -D warnings
|
||||
|
||||
@@ -60,7 +60,7 @@ cargo test --locked
|
||||
cargo build --release --locked
|
||||
```
|
||||
|
||||
Tests additionally need `openssl`, `dbus-daemon` and Python 3. The focused suite covers destination/TLS limits, isolated wallet policy, catalog validation, MCP data, write outcomes, files and setup. The Docker contract is explicit and uses only disposable synthetic data:
|
||||
Tests additionally need `openssl`, `dbus-daemon`, `dbus-run-session`, `gnome-keyring-daemon` and Python 3. The encrypted wallet regression uses a private bus and temporary keyring; it does not access your desktop wallet. The focused suite covers destination/TLS limits, isolated wallet policy, catalog validation, MCP data, write outcomes, files and setup. The Docker contract is explicit and uses only disposable synthetic data:
|
||||
|
||||
```sh
|
||||
python3 scripts/docker_fixture.py start --state /tmp/grocy-mcp-contract
|
||||
|
||||
@@ -10,7 +10,7 @@ lscr.io/linuxserver/grocy@sha256:3c1ce96cb2d0f0c7ea0c2e0b89f5769be7f2f102204fbee
|
||||
|
||||
The scenario uses actual MCP SDK discovery/calls and synthetic credentials supplied only by test code. A frontend-blocking proxy accepts `/api/…` and `/household/api/…`; setup/doctor and workflows made zero requests outside those prefixes. It covers product/unit/location creation and updates, real inventory, stock-entry editing, opening/transfer/consumption/transaction undo, inventory, conversion and stock-unit cascade, shopping add/remove/clear/export, recipe ingredients/nesting/copy/consumption/undo, meal plans/sections, chore execution/undo, task completion/undo/rescheduling, battery charging/undo, equipment/custom definitions/custom-entity and stock-batch values, account-definition read/write exclusion, a PDF upload/readback/download/delete, iCalendar export, record deletion and read-only rejection.
|
||||
|
||||
The small default suite separately checks malformed/traversing destinations, real self-signed TLS rejection, redirects, compression/size caps, interrupted write without replay, plaintext Secret Service refusal against isolated D-Bus, provider identity mismatch, preference ownership/modes/symlinks, field/quantity/name validation, required conversion preflight, MCP result equivalence/annotations, grouped results, successful writes with failed readback, file bytes and setup cancellation/recovery/configuration preservation. The actual desktop wallet's encrypted success path and interactive real enrollment require local use; synthetic tests do not claim to have enrolled a real wallet or household account.
|
||||
The small default suite separately checks malformed/traversing destinations, real self-signed TLS rejection, redirects, compression/size caps, interrupted write without replay, plaintext Secret Service refusal against isolated D-Bus, provider identity mismatch, preference ownership/modes/symlinks, field/quantity/name validation, required conversion preflight, MCP result equivalence/annotations, grouped results, successful writes with failed readback, file bytes and setup cancellation/recovery/configuration preservation. A separate private D-Bus session and temporary GNOME keyring exercise real libsecret encrypted profile save, fresh-session readback, replacement and removal, with GLib critical errors treated as failures. Synthetic tests do not claim to have enrolled a real desktop wallet or household account.
|
||||
|
||||
Final delivery checks use formatting, Clippy with warnings denied, the focused tests, the explicit Docker contract and a release build. The one-time RustSec check against the current advisory database found no reported vulnerable locked dependency (290 dependencies, 2026-10-01). This is a time-specific advisory result, not proof of absence of vulnerabilities.
|
||||
|
||||
|
||||
@@ -26,7 +26,7 @@ Setup detects active and activatable Secret Service providers. With several prov
|
||||
|
||||
If protection cannot be confirmed, the menu offers help, acknowledgment of a password you set, cancellation, or an explicit unsafe-storage override (default off). The override may permit plaintext *wallet storage*. It **never** permits plaintext application-to-wallet communication: the encrypted Secret Service session remains mandatory. The normal path stores the entire private profile, including address, key and household metadata, in your selected wallet.
|
||||
|
||||
Wallet/network recovery keeps answers in memory for retries. Re-running setup reuses the selected provider, saved address/key and access mode, then asks before replacing the profile. A read-only enrollment is available with `setup --read-only`. To enable writes on a previously read-only profile, forget that profile and enroll it again, or use a separate profile (`setup --profile household`).
|
||||
Setup confirms the saved profile by reading it back through a fresh encrypted wallet session before reporting success. Wallet/network recovery keeps answers in memory for retries. Re-running setup reuses the selected provider, saved address/key and access mode, then asks before replacing the profile. A read-only enrollment is available with `setup --read-only`. To enable writes on a previously read-only profile, forget that profile and enroll it again, or use a separate profile (`setup --profile household`).
|
||||
|
||||
## Hermes connection
|
||||
|
||||
|
||||
@@ -19,6 +19,9 @@ fn wallet_error() -> AppError {
|
||||
}
|
||||
pub(crate) struct WalletSession {
|
||||
pub collection: libsecret::Collection,
|
||||
// Collections and items only hold weak references to their service. Keep it
|
||||
// alive for the entire operation, and drop it after the collection.
|
||||
_service: libsecret::Service,
|
||||
pub cancel: gio::Cancellable,
|
||||
done: mpsc::Sender<()>,
|
||||
}
|
||||
@@ -69,6 +72,7 @@ impl WalletSession {
|
||||
}
|
||||
Ok(Self {
|
||||
collection,
|
||||
_service: service,
|
||||
cancel,
|
||||
done,
|
||||
})
|
||||
@@ -212,6 +216,15 @@ pub(crate) fn put(s: ProfileSelection, p: PrivateProfile) -> Result<()> {
|
||||
Some(&session.cancel),
|
||||
)
|
||||
.map_err(|_| wallet_error())?;
|
||||
// Confirm persistence through a fresh encrypted session before setup can
|
||||
// report success. A successful CreateItem alone is not sufficient.
|
||||
let saved = load(s)?;
|
||||
if *encode(&saved)? != *bytes {
|
||||
return Err(AppError::new(
|
||||
"WALLET_SAVE_UNVERIFIED",
|
||||
"The wallet did not confirm the saved profile. Open its native manager and retry; your setup answers are kept.",
|
||||
));
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
pub(crate) fn forget(s: ProfileSelection) -> Result<()> {
|
||||
|
||||
118
tests/wallet_roundtrip.rs
Normal file
118
tests/wallet_roundtrip.rs
Normal file
@@ -0,0 +1,118 @@
|
||||
use hermes_grocy_mcp::{
|
||||
client::ApiBase,
|
||||
credentials::{
|
||||
CredentialStore, DesktopStore, EnrollmentConsent, providers::discover_providers,
|
||||
},
|
||||
model::{AccessMode, PrivateProfile, ProfileSelection, TransportPolicy},
|
||||
};
|
||||
use secrecy::{ExposeSecret, SecretString};
|
||||
use std::{
|
||||
io::Write,
|
||||
process::{Child, Command, Stdio},
|
||||
time::{Duration, Instant},
|
||||
};
|
||||
|
||||
struct WalletDaemon(Child);
|
||||
impl Drop for WalletDaemon {
|
||||
fn drop(&mut self) {
|
||||
let _ = self.0.kill();
|
||||
let _ = self.0.wait();
|
||||
}
|
||||
}
|
||||
|
||||
fn profile(key: &str) -> PrivateProfile {
|
||||
PrivateProfile {
|
||||
api_base: ApiBase::parse("https://grocy.example/api", TransportPolicy::HttpsOnly).unwrap(),
|
||||
api_key: SecretString::from(key.to_owned()),
|
||||
access: AccessMode::Full,
|
||||
timezone: Some("Etc/UTC".into()),
|
||||
defaults: serde_json::json!({}),
|
||||
ca_path: None,
|
||||
transport: TransportPolicy::HttpsOnly,
|
||||
unsafe_storage: false,
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn encrypted_wallet_roundtrip() {
|
||||
if std::env::var_os("GROCY_ENCRYPTED_WALLET_TEST_CHILD").is_none() {
|
||||
// A private bus and data directory ensure this cannot reach the user's wallet.
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let result = Command::new("dbus-run-session")
|
||||
.arg("--")
|
||||
.arg(std::env::current_exe().unwrap())
|
||||
.args(["--exact", "encrypted_wallet_roundtrip", "--nocapture"])
|
||||
.env("GROCY_ENCRYPTED_WALLET_TEST_CHILD", "1")
|
||||
.env("XDG_DATA_HOME", dir.path())
|
||||
.env("XDG_RUNTIME_DIR", dir.path())
|
||||
.env("G_DEBUG", "fatal-criticals")
|
||||
.output()
|
||||
.expect("Install dbus and gnome-keyring to run the isolated wallet test");
|
||||
assert!(
|
||||
result.status.success(),
|
||||
"Isolated wallet roundtrip failed:\n{}\n{}",
|
||||
String::from_utf8_lossy(&result.stdout),
|
||||
String::from_utf8_lossy(&result.stderr)
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
let mut daemon = WalletDaemon(
|
||||
Command::new("gnome-keyring-daemon")
|
||||
.args(["--foreground", "--unlock", "--components=secrets"])
|
||||
.arg("--control-directory")
|
||||
.arg(std::env::var_os("XDG_RUNTIME_DIR").unwrap())
|
||||
.stdin(Stdio::piped())
|
||||
.stdout(Stdio::null())
|
||||
.stderr(Stdio::null())
|
||||
.spawn()
|
||||
.expect("Install gnome-keyring to run the isolated wallet test"),
|
||||
);
|
||||
daemon
|
||||
.0
|
||||
.stdin
|
||||
.take()
|
||||
.unwrap()
|
||||
.write_all(b"synthetic-isolated-wallet-password")
|
||||
.unwrap();
|
||||
let deadline = Instant::now() + Duration::from_secs(10);
|
||||
let provider = loop {
|
||||
if let Ok(providers) = discover_providers()
|
||||
&& let Some(identity) = providers.into_iter().find_map(|p| p.identity)
|
||||
{
|
||||
break identity;
|
||||
}
|
||||
assert!(Instant::now() < deadline, "Isolated wallet did not start");
|
||||
std::thread::sleep(Duration::from_millis(50));
|
||||
};
|
||||
let selection = ProfileSelection {
|
||||
profile: "synthetic-roundtrip".into(),
|
||||
provider,
|
||||
};
|
||||
tokio::runtime::Runtime::new().unwrap().block_on(async {
|
||||
assert!(matches!(
|
||||
DesktopStore.load(selection.clone()).await,
|
||||
Err(e) if e.code == "PROFILE_NOT_FOUND"
|
||||
));
|
||||
for key in ["synthetic-first-key", "synthetic-replacement-key"] {
|
||||
DesktopStore
|
||||
.put(
|
||||
selection.clone(),
|
||||
profile(key),
|
||||
EnrollmentConsent::ProtectedByUser,
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
let loaded = DesktopStore.load(selection.clone()).await.unwrap();
|
||||
assert!(loaded.api_key.expose_secret() == key);
|
||||
assert!(loaded.api_base.as_str() == profile(key).api_base.as_str());
|
||||
assert_eq!(loaded.access, AccessMode::Full);
|
||||
assert!(!loaded.unsafe_storage);
|
||||
}
|
||||
DesktopStore.forget(selection.clone()).await.unwrap();
|
||||
assert!(matches!(
|
||||
DesktopStore.load(selection).await,
|
||||
Err(e) if e.code == "PROFILE_NOT_FOUND"
|
||||
));
|
||||
});
|
||||
}
|
||||
Reference in New Issue
Block a user