Skip to content

Commit 8f4ce10

Browse files
author
Michael Feth
committed
fix(desktop): bound the device stamp to local agents and validate every label
Review follow-up. Four issues, all confirmed against the code first. **A provider-backed agent was given the wrong location.** `agent_event_content` stamped the current device onto every record without consulting `record.backend`. A `Provider` body runs elsewhere -- deployed to a cluster from a laptop -- and stays online after that laptop sleeps, so the mention UI told the user "only that device can reply" about a machine that is not where the agent runs. The stamp is now gated on `BackendKind::Local`. That alone closes the false guidance: `describeUnrunnableMention` already returns `null` when no `device_id` is published, so such an agent now produces no notice at all rather than a corrected one. Distinguishing key *custody* from *execution* is a protocol change, not a second stamp, and is left as a named future-work note. **The label contract was bypassed on stored and inbound data.** Validation ran only when a label was typed. Three holes: - `load_or_create_at` returned whatever deserialized. A hand-edited `device.json` was published unchecked. It now validates the whole identity -- id shape and label -- and an invalid file takes the path a corrupt one already did: preserved as `device.json.corrupt`, replaced, never failing the caller. - The inbound kind:30177 projection forwarded `device_id`/`device_label` verbatim. Owner authentication proves authorship, not well-formedness; a sibling device on an older or tampered build can sign anything. Each field is now validated independently and degrades to `None`, so a bad label never hides an otherwise reachable agent. - `char::is_control` is category `Cc` only, so zero-width (U+200B) and bidi overrides (U+202E) passed through. Rather than a second rule, the label now goes through `validate_device_label`, reusing the visible-text policy that already guards agent definition text -- whose own doc comment asks for validation "at every local, inbound, and publication boundary". An over-long label is now refused rather than truncated: publishing something other than what the owner typed is the worse failure. **Rename claimed more than it did.** It reconciles only the active retention scope. Republishing every scope needs owner keys for communities that are not applied -- identity handling, not this command -- so the contract is now stated truthfully instead: immediate for the active community, eventual elsewhere via `run_event_sync` on activation. **The OS host name could be published before the owner saw the warning.** First-run labels are now opaque (`device-<8 hex>`) and `mint_identity` no longer reads the host name. It is offered as an explicit opt-in in the settings card via `get_device_name_suggestion`, so nothing derived from it reaches a relay until the owner applies it. The new tests share a process-global cache, so the seam is an RAII guard holding a mutex and restoring on drop -- deliberately not repeating the pattern that makes `claude_spawn_uses_the_probed_cli_executable` flaky. Signed-off-by: Michael Feth <michael@jira-flow.com>
1 parent 23c95ff commit 8f4ce10

11 files changed

Lines changed: 562 additions & 67 deletions

File tree

desktop/src-tauri/src/device_identity.rs

Lines changed: 251 additions & 59 deletions
Large diffs are not rendered by default.

desktop/src-tauri/src/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -784,6 +784,7 @@ pub fn run() {
784784
set_global_agent_config,
785785
device_identity::get_device_identity,
786786
device_identity::set_device_label,
787+
device_identity::get_device_name_suggestion,
787788
mesh_start_node,
788789
mesh_stop_node,
789790
mesh_node_status,

desktop/src-tauri/src/managed_agents/agent_events.rs

Lines changed: 68 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,23 @@ pub fn agent_event_content(record: &ManagedAgentRecord) -> ManagedAgentEventCont
9292
// Device fields describe the INSTANCE (which install holds its secret),
9393
// never the definition, so they are emitted regardless of slimming.
9494
// `None` before the Tauri setup hook runs — unit tests publish no stamp.
95-
let device = crate::device_identity::current();
95+
//
96+
// Only a LOCAL backend is device-bound. A `Provider` backend's body runs
97+
// elsewhere — deployed to Kubernetes from a laptop, say — and stays online
98+
// after this install sleeps, so stamping it with this Desktop would make
99+
// the mention UI claim "only that device can reply" about a machine that is
100+
// not where the agent runs. Such a record publishes no device at all and
101+
// degrades to the same "no device information" rendering as a pre-Stage-0
102+
// peer.
103+
//
104+
// Future work (out of scope for Stage 0): a provider-backed agent still has
105+
// a *custody* device — the install holding its secret — which is a
106+
// different coordinate from its *execution* location. Distinguishing the
107+
// two needs a protocol change, not a second stamp here.
108+
let device = match record.backend {
109+
super::BackendKind::Local => crate::device_identity::current(),
110+
super::BackendKind::Provider { .. } => None,
111+
};
96112
ManagedAgentEventContent {
97113
name: record.name.clone(),
98114
persona_id: record.persona_id.clone(),
@@ -480,6 +496,9 @@ mod tests {
480496
/// device fields existed. This is why no other test in the crate changed.
481497
#[test]
482498
fn projection_omits_device_fields_without_a_device_identity() {
499+
// Takes the guard (with `None`) purely to serialize against the tests
500+
// below that seed a device — `CURRENT` is process-global.
501+
let _guard = crate::device_identity::DeviceGuard::set(None);
483502
assert!(
484503
crate::device_identity::current().is_none(),
485504
"unit tests must never boot the device identity"
@@ -495,6 +514,54 @@ mod tests {
495514
assert!(!json.contains("device_label"), "{json}");
496515
}
497516

517+
/// A local-backend agent's secret lives on this install, so it is the one
518+
/// case where naming this computer is true.
519+
#[test]
520+
fn projection_stamps_the_device_for_a_local_backend() {
521+
use crate::device_identity::DeviceGuard;
522+
let device = DeviceGuard::sample();
523+
let _guard = DeviceGuard::set(Some(device.clone()));
524+
525+
let mut record = sample_agent();
526+
record.backend = super::super::BackendKind::Local;
527+
528+
let content = agent_event_content(&record);
529+
assert_eq!(
530+
content.device_id.as_deref(),
531+
Some(device.device_id.as_str())
532+
);
533+
assert_eq!(
534+
content.device_label.as_deref(),
535+
Some(device.device_label.as_str())
536+
);
537+
}
538+
539+
/// A provider-backed agent's body runs elsewhere and outlives this install,
540+
/// so stamping it here would make the mention UI claim "only that device can
541+
/// reply" about a machine that is not where the agent runs.
542+
#[test]
543+
fn projection_omits_the_device_for_a_provider_backend() {
544+
use crate::device_identity::DeviceGuard;
545+
let _guard = DeviceGuard::set(Some(DeviceGuard::sample()));
546+
547+
let mut record = sample_agent();
548+
record.backend = super::super::BackendKind::Provider {
549+
id: "buzz-backend-x".to_string(),
550+
config: serde_json::json!({ "cluster": "staging" }),
551+
};
552+
553+
let content = agent_event_content(&record);
554+
assert_eq!(
555+
content.device_id, None,
556+
"a remote body must not be given this computer's id"
557+
);
558+
assert_eq!(content.device_label, None);
559+
560+
let json = serde_json::to_string(&content).unwrap();
561+
assert!(!json.contains("deviceId"), "{json}");
562+
assert!(!json.contains("deviceLabel"), "{json}");
563+
}
564+
498565
/// Mixed-fleet back-compat: a 30177 event published by a build that predates
499566
/// device identity parses cleanly, with both fields absent rather than an
500567
/// invented value.

desktop/src-tauri/src/managed_agents/definition_validation.rs

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,32 @@ pub(crate) fn validate_managed_agent_definition_text(
6060
validate_agent_definition_text(name, executable_prompt)
6161
}
6262

63+
/// Maximum length of a device label, in `char`s.
64+
pub(crate) const MAX_DEVICE_LABEL_CHARS: usize = 32;
65+
66+
/// Validate a device label against the same visible-text policy as agent
67+
/// definition text.
68+
///
69+
/// A device label names the computer an agent lives on and is published in a
70+
/// world-readable kind:30177 event, then rendered beside an agent's name in
71+
/// other people's clients. That makes it the same class of input as a display
72+
/// name: `char::is_control` alone would pass zero-width characters (U+200B) and
73+
/// bidi overrides (U+202E), which are Unicode category `Cf` and can visually
74+
/// reorder the text around them.
75+
pub(crate) fn validate_device_label(label: &str) -> Result<(), String> {
76+
let trimmed = label.trim();
77+
if trimmed.is_empty() {
78+
return Err("Device name must not be empty".to_string());
79+
}
80+
let count = trimmed.chars().count();
81+
if count > MAX_DEVICE_LABEL_CHARS {
82+
return Err(format!(
83+
"Device name is too long ({count} characters, max {MAX_DEVICE_LABEL_CHARS})"
84+
));
85+
}
86+
validate_visible_text(trimmed, "Device name", false)
87+
}
88+
6389
fn validate_visible_text(
6490
value: &str,
6591
label: &str,

desktop/src-tauri/src/managed_agents/mod.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,9 @@ pub(crate) use agent_env::{
1111
mod backend;
1212
pub(crate) mod config_bridge;
1313
pub(crate) mod custom_harnesses;
14-
mod definition_validation;
14+
// `pub(crate)` so the device-identity and inbound-directory paths can reuse the
15+
// one visible-text policy instead of growing a second, drifting copy.
16+
pub(crate) mod definition_validation;
1517
mod discovery;
1618
pub(crate) mod effective_config;
1719
mod env_vars;

desktop/src-tauri/src/nostr_convert/agent_directory.rs

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ use std::collections::{BTreeSet, HashMap};
44

55
use nostr::Event;
66

7+
use crate::device_identity::validate_device_id;
8+
use crate::managed_agents::definition_validation::validate_device_label;
79
use crate::managed_agents::{agent_events::managed_agent_content_from_event, RelayAgentInfo};
810

911
use super::{agents_from_events, first_tag_value, profile_valid_oa_owner_pubkey, tags_named};
@@ -138,8 +140,22 @@ fn relay_agent_from_managed_policy(agent_pubkey: &str, event: &Event) -> Option<
138140
status: "offline".to_string(),
139141
respond_to: Some(content.respond_to),
140142
respond_to_allowlist: content.respond_to_allowlist,
141-
device_id: content.device_id,
142-
device_label: content.device_label,
143+
// Owner authentication proves *who wrote this*, not that what they
144+
// wrote is well-formed. A sibling device running an older, buggy, or
145+
// tampered-with build can publish any string here, and these two values
146+
// are rendered verbatim beside an agent's name — so they are validated
147+
// like any other untrusted input before they reach the UI.
148+
//
149+
// A value that fails degrades to `None` on its own. Dropping the whole
150+
// directory entry over a bad label would hide a real, reachable agent;
151+
// dropping just the label falls back to the same "no device
152+
// information" rendering as a peer that predates Stage 0.
153+
device_id: content
154+
.device_id
155+
.filter(|id| validate_device_id(id).is_ok()),
156+
device_label: content
157+
.device_label
158+
.filter(|label| validate_device_label(label).is_ok()),
143159
})
144160
}
145161

desktop/src-tauri/src/nostr_convert/tests.rs

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -496,6 +496,73 @@ fn managed_agent_directory_surfaces_the_owner_verified_device_label() {
496496
assert_eq!(agents[0].device_label, None);
497497
}
498498

499+
/// Owner authentication proves authorship, not well-formedness: a sibling
500+
/// device on an older, buggy, or tampered build can sign anything. A bad value
501+
/// must degrade to `None` on its own without hiding a real, reachable agent.
502+
#[test]
503+
fn managed_agent_directory_drops_invalid_device_metadata_but_keeps_the_agent() {
504+
let agent_keys = Keys::generate();
505+
let owner_keys = Keys::generate();
506+
let agent_pubkey = agent_keys.public_key().to_hex();
507+
508+
let auth_tag_json =
509+
buzz_sdk_pkg::nip_oa::compute_auth_tag(&owner_keys, &agent_keys.public_key(), "")
510+
.expect("compute auth tag");
511+
let auth_tag_values: Vec<String> =
512+
serde_json::from_str(&auth_tag_json).expect("parse auth tag json");
513+
let profile = EventBuilder::new(Kind::Metadata, r#"{"display_name":"Bumble"}"#)
514+
.tags([Tag::parse(auth_tag_values).expect("parse auth tag")])
515+
.sign_with_keys(&agent_keys)
516+
.expect("sign profile");
517+
518+
// A bidi override in the label and a malformed id, both correctly signed.
519+
let hostile = EventBuilder::new(
520+
Kind::Custom(30177),
521+
serde_json::json!({
522+
"name": "Bumble",
523+
"parallelism": 1,
524+
"respond_to": "anyone",
525+
"device_id": "not-a-uuid",
526+
"device_label": "mfeth\u{202E}win",
527+
})
528+
.to_string(),
529+
)
530+
.tags([Tag::parse(["d", agent_pubkey.as_str()]).expect("parse d tag")])
531+
.sign_with_keys(&owner_keys)
532+
.expect("sign managed-agent event");
533+
534+
let agents = relay_agents_from_managed_agent_events(&[hostile], std::slice::from_ref(&profile));
535+
assert_eq!(agents.len(), 1, "the agent itself must still be reachable");
536+
assert_eq!(agents[0].name, "Bumble");
537+
assert_eq!(agents[0].device_id, None, "malformed id dropped");
538+
assert_eq!(agents[0].device_label, None, "bidi-bearing label dropped");
539+
540+
// An over-long label is refused by the same policy.
541+
let long = EventBuilder::new(
542+
Kind::Custom(30177),
543+
serde_json::json!({
544+
"name": "Bumble",
545+
"parallelism": 1,
546+
"respond_to": "anyone",
547+
"device_id": "0123456789abcdef0123456789abcdef",
548+
"device_label": "a".repeat(33),
549+
})
550+
.to_string(),
551+
)
552+
.tags([Tag::parse(["d", agent_pubkey.as_str()]).expect("parse d tag")])
553+
.sign_with_keys(&owner_keys)
554+
.expect("sign managed-agent event");
555+
556+
let agents = relay_agents_from_managed_agent_events(&[long], std::slice::from_ref(&profile));
557+
assert_eq!(agents.len(), 1);
558+
assert_eq!(
559+
agents[0].device_id.as_deref(),
560+
Some("0123456789abcdef0123456789abcdef"),
561+
"a valid id survives its label being dropped"
562+
);
563+
assert_eq!(agents[0].device_label, None);
564+
}
565+
499566
#[test]
500567
fn managed_agent_directory_rejects_agents_without_verified_owner_profiles() {
501568
let owner_keys = Keys::generate();

desktop/src/features/settings/ui/DeviceNameSettingsCard.tsx

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,10 @@ import {
66
deviceIdentityQueryKey,
77
useDeviceIdentityQuery,
88
} from "@/features/agents/hooks";
9-
import { setDeviceLabel } from "@/shared/api/tauriDeviceIdentity";
9+
import {
10+
getDeviceNameSuggestion,
11+
setDeviceLabel,
12+
} from "@/shared/api/tauriDeviceIdentity";
1013
import { Button } from "@/shared/ui/button";
1114
import { Input } from "@/shared/ui/input";
1215
import { SettingsOptionGroup, SettingsOptionRow } from "./SettingsOptionGroup";
@@ -51,7 +54,33 @@ export function DeviceNameSettingsCard() {
5154
},
5255
});
5356

57+
// The OS host name is only ever *offered*. A device's name starts opaque
58+
// (`device-xxxxxxxx`) precisely because host names routinely carry a real
59+
// person's name and this label is published world-readable — so the owner
60+
// opts in here, seeing the warning above before anything leaves the machine.
61+
const [suggestion, setSuggestion] = React.useState<string | null>(null);
62+
React.useEffect(() => {
63+
let cancelled = false;
64+
void getDeviceNameSuggestion()
65+
.then((value) => {
66+
if (!cancelled) {
67+
setSuggestion(value);
68+
}
69+
})
70+
.catch(() => {
71+
// Advisory only — a device with no usable host name simply gets no
72+
// suggestion, which is not worth surfacing as an error.
73+
});
74+
return () => {
75+
cancelled = true;
76+
};
77+
}, []);
78+
5479
const trimmedLabel = draftLabel.trim();
80+
const showSuggestion =
81+
suggestion !== null &&
82+
suggestion !== savedLabel &&
83+
suggestion !== trimmedLabel;
5584
const saveDisabled =
5685
trimmedLabel.length === 0 ||
5786
trimmedLabel === savedLabel ||
@@ -74,6 +103,19 @@ export function DeviceNameSettingsCard() {
74103
other devices. Published with your agents, so avoid personal
75104
details.
76105
</p>
106+
{showSuggestion ? (
107+
<button
108+
className="mt-1 text-xs text-muted-foreground underline underline-offset-2 hover:text-foreground"
109+
data-testid="device-name-use-hostname"
110+
onClick={() => {
111+
setEditedLabel(true);
112+
setDraftLabel(suggestion);
113+
}}
114+
type="button"
115+
>
116+
Use this computer's name ({suggestion})
117+
</button>
118+
) : null}
77119
</div>
78120
<div className="flex shrink-0 items-center gap-2">
79121
<Input

desktop/src/shared/api/tauriDeviceIdentity.ts

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,24 @@ export async function getDeviceIdentity(): Promise<DeviceIdentity> {
99
/**
1010
* Rename this device.
1111
*
12-
* The backend trims, rejects control characters, caps the label at 32
13-
* characters, and republishes the label on every local agent's kind:30177
14-
* event, so callers need no follow-up write.
12+
* The backend trims and rejects an empty, over-long (>32 char), or
13+
* invisible-character-bearing label — including the zero-width and bidi
14+
* codepoints `char::is_control` misses — then republishes the label on the
15+
* active community's local agents, so callers need no follow-up write. The
16+
* owner's other communities pick it up when next activated.
1517
*/
1618
export async function setDeviceLabel(label: string): Promise<DeviceIdentity> {
1719
return invokeTauri<DeviceIdentity>("set_device_label", { label });
1820
}
21+
22+
/**
23+
* The OS host name, offered as a suggested device name.
24+
*
25+
* `null` when the host name is unusable under the label policy. This is only a
26+
* suggestion: a device's name starts opaque (`device-xxxxxxxx`) and the host
27+
* name — which routinely contains a real person's name — is never published
28+
* until the owner applies it.
29+
*/
30+
export async function getDeviceNameSuggestion(): Promise<string | null> {
31+
return invokeTauri<string | null>("get_device_name_suggestion");
32+
}

desktop/src/testing/e2eBridge.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3345,6 +3345,9 @@ const DEFAULT_MOCK_DEVICE_IDENTITY = {
33453345
createdAt: "2026-01-01T00:00:00Z",
33463346
};
33473347
let mockDeviceIdentity = { ...DEFAULT_MOCK_DEVICE_IDENTITY };
3348+
// Stands in for the OS host name the settings card offers as an opt-in. Kept
3349+
// distinct from MOCK_DEVICE_LABEL so a test can tell "suggested" from "applied".
3350+
const MOCK_HOSTNAME_SUGGESTION = "marys-macbook";
33483351

33493352
const defaultMockRelayAgents: RawRelayAgent[] = [
33503353
{
@@ -12285,6 +12288,8 @@ export function maybeInstallE2eTauriMocks() {
1228512288
}
1228612289
case "get_device_identity":
1228712290
return { ...mockDeviceIdentity };
12291+
case "get_device_name_suggestion":
12292+
return MOCK_HOSTNAME_SUGGESTION;
1228812293
case "set_device_label": {
1228912294
const label = (payload as { label?: unknown } | undefined)?.label;
1229012295
if (typeof label === "string" && label.trim().length > 0) {

0 commit comments

Comments
 (0)