Skip to content

Commit 9fcd0e7

Browse files
Michael Fethmfethe1
authored andcommitted
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 1bbab21 commit 9fcd0e7

16 files changed

Lines changed: 634 additions & 229 deletions

File tree

desktop/src-tauri/src/commands/agent_discovery/relay_directory.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -261,6 +261,8 @@ mod tests {
261261
status: "offline".to_string(),
262262
respond_to: None,
263263
respond_to_allowlist: Vec::new(),
264+
device_id: None,
265+
device_label: None,
264266
},
265267
RelayAgentInfo {
266268
pubkey: "c".repeat(64),
@@ -273,6 +275,8 @@ mod tests {
273275
status: "online".to_string(),
274276
respond_to: None,
275277
respond_to_allowlist: Vec::new(),
278+
device_id: None,
279+
device_label: None,
276280
},
277281
];
278282

@@ -299,6 +303,8 @@ mod tests {
299303
status: "online".to_string(),
300304
respond_to: None,
301305
respond_to_allowlist: Vec::new(),
306+
device_id: None,
307+
device_label: None,
302308
}];
303309

304310
retain_agents_allowed_by_build(&mut agents, false);

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
@@ -713,6 +713,7 @@ pub fn run() {
713713
set_global_agent_config,
714714
device_identity::get_device_identity,
715715
device_identity::set_device_label,
716+
device_identity::get_device_name_suggestion,
716717
mesh_start_node,
717718
mesh_stop_node,
718719
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(),
@@ -483,6 +499,9 @@ mod tests {
483499
/// device fields existed. This is why no other test in the crate changed.
484500
#[test]
485501
fn projection_omits_device_fields_without_a_device_identity() {
502+
// Takes the guard (with `None`) purely to serialize against the tests
503+
// below that seed a device — `CURRENT` is process-global.
504+
let _guard = crate::device_identity::DeviceGuard::set(None);
486505
assert!(
487506
crate::device_identity::current().is_none(),
488507
"unit tests must never boot the device identity"
@@ -498,6 +517,54 @@ mod tests {
498517
assert!(!json.contains("device_label"), "{json}");
499518
}
500519

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

82+
/// Maximum length of a device label, in `char`s.
83+
pub(crate) const MAX_DEVICE_LABEL_CHARS: usize = 32;
84+
85+
/// Validate a device label against the same visible-text policy as agent
86+
/// definition text.
87+
///
88+
/// A device label names the computer an agent lives on and is published in a
89+
/// world-readable kind:30177 event, then rendered beside an agent's name in
90+
/// other people's clients. That makes it the same class of input as a display
91+
/// name: `char::is_control` alone would pass zero-width characters (U+200B) and
92+
/// bidi overrides (U+202E), which are Unicode category `Cf` and can visually
93+
/// reorder the text around them.
94+
pub(crate) fn validate_device_label(label: &str) -> Result<(), String> {
95+
let trimmed = label.trim();
96+
if trimmed.is_empty() {
97+
return Err("Device name must not be empty".to_string());
98+
}
99+
let count = trimmed.chars().count();
100+
if count > MAX_DEVICE_LABEL_CHARS {
101+
return Err(format!(
102+
"Device name is too long ({count} characters, max {MAX_DEVICE_LABEL_CHARS})"
103+
));
104+
}
105+
validate_visible_text(trimmed, "Device name", false)
106+
}
107+
82108
/// Reject control and default-ignorable characters in human-reviewed text.
83109
///
84110
/// The shared executable-definition invariant: a recipient reviews a visible

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,9 @@ mod backend;
1414
pub(crate) mod claude_config;
1515
pub(crate) mod config_bridge;
1616
pub(crate) mod custom_harnesses;
17-
mod definition_validation;
17+
// `pub(crate)` so the device-identity and inbound-directory paths can reuse the
18+
// one visible-text policy instead of growing a second, drifting copy.
19+
pub(crate) mod definition_validation;
1820
mod discovery;
1921
pub(crate) mod effective_config;
2022
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/messages/lib/mentionSuggestionMapping.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ export type MentionSuggestionCandidate = {
2020
role?: ChannelRole | null;
2121
ownerPubkey?: string | null;
2222
deviceLabel?: string | null;
23-
isManagedAgent?: boolean;
2423
};
2524

2625
export function mapMentionCandidateToSuggestion(opts: {

desktop/src/features/messages/ui/MentionAutocomplete.tsx

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -274,13 +274,22 @@ export const MentionAutocomplete = React.memo(function MentionAutocomplete({
274274
// Same account on several computers mints one keypair per
275275
// computer, so identically named agents are usually different
276276
// devices.
277-
const deviceLine = suggestion.isAgent
278-
? describeAgentDevice({
279-
isLocal: suggestion.isLocalAgent === true,
280-
deviceLabel: suggestion.deviceLabel,
281-
hasNameCollision,
282-
})
283-
: null;
277+
// Only speak when we actually know something about the device.
278+
// A suggestion carrying neither a label nor a local/remote verdict
279+
// (upstream fixtures, teams, pre-feature agents) must stay silent:
280+
// guessing "on another device" is the same false statement
281+
// describeUnrunnableMention already refuses to make.
282+
const hasDeviceKnowledge =
283+
suggestion.deviceLabel != null ||
284+
suggestion.isLocalAgent !== undefined;
285+
const deviceLine =
286+
suggestion.isAgent && hasDeviceKnowledge
287+
? describeAgentDevice({
288+
isLocal: suggestion.isLocalAgent === true,
289+
deviceLabel: suggestion.deviceLabel,
290+
hasNameCollision,
291+
})
292+
: null;
284293
const hasMetadataBeforeNpub = Boolean(
285294
suggestion.kind === "team" ||
286295
suggestion.isAgent ||

0 commit comments

Comments
 (0)