sassycrown/punktfunk
1
0
Fork 0
forked from unom/punktfunk
Code Pull Requests Activity

fix(cli): hosts add --fp fills in an empty fingerprint instead of dropping it

Browse Source 
`punktfunk hosts add <addr> --fp <hex>` against an address already in the store printed
"is already saved" and exited 0 — having done nothing at all. The --fp was silently
discarded, so a host saved by address stayed pinless and every later connect refused
for want of a fingerprint, with no line anywhere saying why.

Three outcomes now, and the difference between them is a trust decision:

  • no fingerprint on the record, one offered  → fill it in, print `updated <addr>:<port>`
  • the same fingerprint offered again        → no-op, exit 0 (a panel may retry a step
                                                whose state is already correct without
                                                having to invent an error to show)
  • a DIFFERENT fingerprint                    → refuse, exit 3

The refusal is the important one. A changed identity is a decision for a person at a
surface that can show them both — the rule `upsert_trusted` exists to enforce — and
quietly overwriting a pin here would be a back door through the pinning the rest of the
client is built on.

A record still named after its own address takes an offered --name; a label the user
chose is theirs and an advert's name must not overwrite it.
This commit is contained in:
enricobuehler
2026-08-04 20:28:11 +02:00
parent 48bb1769b4
commit aec02b9d26
1 changed files with 159 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files
clients/cli/src/main.rs
+159 -5
View File
@@ -538,16 +538,44 @@ from the config directory for a true factory reset."
return UNRESOLVED;
};
let (addr, port) = split_host_port(&target);
let fp = value(args, "--fp").unwrap_or_default();
let name = value(args, "--name");
let mut known = KnownHosts::load();
if known.hosts.iter().any(|h| h.addr == addr && h.port == port) {
eprintln!("{addr}:{port} is already saved");
return OK;
if let Some(i) = known
.hosts
.iter()
.position(|h| h.addr == addr && h.port == port)
{
return match merge_saved_host(&mut known, i, &fp, name.as_deref()) {
AddOutcome::Unchanged => {
eprintln!("{addr}:{port} is already saved");
OK
}
AddOutcome::Conflict => {
eprintln!(
"{addr}:{port} is already saved with a different fingerprint — \
forget it first if you really mean to replace it \
(punktfunk hosts forget {addr}:{port})"
);
TRUST_REJECTED
}
AddOutcome::Pinned => match known.save() {
Ok(()) => {
println!("updated {addr}:{port}");
OK
}
Err(e) => {
eprintln!("saving: {e:#}");
CONNECT_FAILED
}
},
};
}
known.hosts.push(KnownHost {
name: value(args, "--name").unwrap_or_else(|| addr.clone()),
name: name.unwrap_or_else(|| addr.clone()),
addr: addr.clone(),
port,
fp_hex: value(args, "--fp").unwrap_or_default(),
fp_hex: fp,
..Default::default()
});
match known.save() {
@@ -589,6 +617,55 @@ from the config directory for a true factory reset."
}
}
/// What `hosts add` did to a record that was ALREADY saved for this address.
#[derive(Debug, PartialEq, Eq)]
enum AddOutcome {
/// Nothing to do — no fingerprint was offered, or the record already carries this one.
/// Exits 0 on purpose: a panel retrying step 1 of request access must not have to
/// invent an error to show for a state that is already correct.
Unchanged,
/// The record had no fingerprint and now has this one.
Pinned,
/// The record carries a DIFFERENT fingerprint. Refused, never overwritten.
Conflict,
}
/// `hosts add --fp` against an address that is already saved. The difference between these
/// three is a trust decision, not bookkeeping.
///
/// Filling in an empty fingerprint is step 1 of request access (design §5): a host found by
/// advert is saved by address first and pinned second. Without it the `--fp` is dropped on
/// the floor and the launch that follows refuses for want of a pin — which is what this did
/// before, silently and with exit 0.
///
/// A *different* fingerprint is refused because a changed identity is a decision for a
/// person, at a surface that can show them both. That is what `upsert_trusted` exists to
/// enforce; quietly overwriting it here would be a back door through the pinning the rest
/// of the client is built on.
fn merge_saved_host(
known: &mut KnownHosts,
i: usize,
fp: &str,
name: Option<&str>,
) -> AddOutcome {
let existing = known.hosts[i].fp_hex.clone();
if fp.is_empty() || existing.eq_ignore_ascii_case(fp) {
return AddOutcome::Unchanged;
}
if !existing.is_empty() {
return AddOutcome::Conflict;
}
known.hosts[i].fp_hex = fp.to_string();
// Only a record still named after its own address is renamed: a label the user chose is
// theirs, and an advert's name must not quietly overwrite it.
if let Some(label) = name {
if known.hosts[i].name == known.hosts[i].addr {
known.hosts[i].name = label.to_string();
}
}
AddOutcome::Pinned
}
/// `wake <host-ref> [--wait]` — a magic packet, and with `--wait` the same bounded
/// wake-and-wait the shells run (`WakeWait`: a packet every 6 s, presence polled every
/// second, 90 s budget).
@@ -1103,6 +1180,83 @@ from the config directory for a true factory reset."
assert!(verb_help("bogus").is_none());
}
fn saved(name: &str, addr: &str, fp: &str) -> KnownHost {
KnownHost {
name: name.into(),
addr: addr.into(),
port: 9777,
fp_hex: fp.into(),
..Default::default()
}
}
/// Step 1 of request access: a host saved by address gains the fingerprint its advert
/// carried. Before this, `hosts add --fp` on an existing record exited 0 having done
/// NOTHING — the launch that followed then refused for want of a pin, and the panel had
/// no way to tell why.
#[test]
fn adding_a_fingerprint_to_a_placeholder_fills_it_in() {
let mut known = KnownHosts {
hosts: vec![saved("192.168.1.9", "192.168.1.9", "")],
};
assert_eq!(
merge_saved_host(&mut known, 0, "abc123", Some("living-room")),
AddOutcome::Pinned
);
assert_eq!(known.hosts[0].fp_hex, "abc123");
assert_eq!(
known.hosts[0].name, "living-room",
"a record still named after its address takes the offered label"
);
}
/// A label the user chose is theirs — an advert's name must not overwrite it.
#[test]
fn filling_in_a_fingerprint_keeps_a_user_chosen_name() {
let mut known = KnownHosts {
hosts: vec![saved("Basement rig", "192.168.1.9", "")],
};
merge_saved_host(&mut known, 0, "abc123", Some("living-room"));
assert_eq!(known.hosts[0].name, "Basement rig");
}
/// Idempotent: the panel may retry step 1, and re-offering the fingerprint a record
/// already carries is a state that is already correct, not an error to render.
#[test]
fn re_adding_the_same_fingerprint_changes_nothing() {
let mut known = KnownHosts {
hosts: vec![saved("desk", "192.168.1.9", "ABC123")],
};
assert_eq!(
merge_saved_host(&mut known, 0, "abc123", None),
AddOutcome::Unchanged,
"fingerprints compare case-insensitively"
);
// And a bare `hosts add` with no --fp at all leaves the pin alone.
assert_eq!(
merge_saved_host(&mut known, 0, "", None),
AddOutcome::Unchanged
);
assert_eq!(known.hosts[0].fp_hex, "ABC123");
}
/// A changed identity is a decision for a person. Never a silent overwrite — this is the
/// same rule `upsert_trusted` enforces, and a back door here would defeat it everywhere.
#[test]
fn a_different_fingerprint_is_refused_not_overwritten() {
let mut known = KnownHosts {
hosts: vec![saved("desk", "192.168.1.9", "abc123")],
};
assert_eq!(
merge_saved_host(&mut known, 0, "deadbeef", None),
AddOutcome::Conflict
);
assert_eq!(
known.hosts[0].fp_hex, "abc123",
"the pin must survive intact"
);
}
#[test]
fn value_reads_the_argument_after_its_flag() {
let a = argv(&["--game", "steam:570", "--exec"]);