diff --git a/AGENTS.md b/AGENTS.md index 9f81f4a..0c8acb7 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -200,8 +200,9 @@ Roadmap items should be actionable and checkable: SSH certificate-flow metadata with `ssh_cert.sync` on `resource:ssh:certs` and revocation metadata with `ssh_revocation.sync` on `resource:ssh:revocations`. The daemon live-syncs known peers every 30 - seconds using per-peer cursors; this is pull-only metadata sync, not yet a - CRDT/resource-log replication model. + seconds using per-peer cursors. Conflicting records with already-known ids are + rejected during sync import; this is pull-only metadata sync, not yet a + signed CRDT/resource-log replication model. - cr-sqlite apply, iroh-docs, iroh-blobs provider/fetch, Automerge sync, broader auth enforcement, and Keyhive/BeeKEM-style authorization are future roadmap items unless implemented later. diff --git a/README.md b/README.md index 0e84a44..a6c5f66 100644 --- a/README.md +++ b/README.md @@ -169,7 +169,8 @@ authorized peer metadata into the local store for offline listing and later approval/signing workflows. While the daemon is running, it also performs a background live-sync tick for known peers every 30 seconds. Live-sync stores per-peer high-water cursors in local metadata so repeated ticks request only -newer SSH certificate-flow and revocation records. +newer SSH certificate-flow and revocation records. Sync import preserves local +metadata by rejecting conflicting records with ids that already exist locally. Before probing individual modules, the daemon asks the peer for an authorized sync-status summary over Iroh. The peer only returns stream watermarks for resources where the caller already has the matching capability, letting the diff --git a/crates/geth-node/src/lib.rs b/crates/geth-node/src/lib.rs index 855768d..7188de1 100644 --- a/crates/geth-node/src/lib.rs +++ b/crates/geth-node/src/lib.rs @@ -910,13 +910,17 @@ async fn ssh_cert_sync_from_peer( }); } let store = Store::open(&node.paths.metadata_db())?; - let requests_imported = requests.len(); - let certificates_imported = certificates.len(); + let mut requests_imported = 0; + let mut certificates_imported = 0; for request in &requests { - store.insert_ssh_cert_request(&stored_from_ssh_cert_request(request))?; + if insert_ssh_cert_request_if_not_conflicting(&store, request)? { + requests_imported += 1; + } } for certificate in &certificates { - store.insert_ssh_certificate(&stored_from_ssh_certificate(certificate))?; + if insert_ssh_certificate_if_not_conflicting(&store, certificate)? { + certificates_imported += 1; + } } store_live_sync_cursor(&store, peer_node, "ssh-certs", high_water_ms)?; Ok(ControlResponse::SshCertSynced { @@ -988,9 +992,11 @@ async fn ssh_revocation_sync_from_peer( }); } let store = Store::open(&node.paths.metadata_db())?; - let revocations_imported = revocations.len(); + let mut revocations_imported = 0; for revocation in &revocations { - store.insert_ssh_revocation(&stored_from_ssh_revocation(revocation))?; + if insert_ssh_revocation_if_not_conflicting(&store, revocation)? { + revocations_imported += 1; + } } store_live_sync_cursor(&store, peer_node, "ssh-revocations", high_water_ms)?; Ok(ControlResponse::SshRevocationSynced { @@ -4792,6 +4798,24 @@ fn stored_from_ssh_cert_request(request: &SshCertRequest) -> StoredSshCertReques } } +fn insert_ssh_cert_request_if_not_conflicting( + store: &Store, + request: &SshCertRequest, +) -> Result { + let stored = stored_from_ssh_cert_request(request); + if let Some(existing) = store.get_ssh_cert_request(&stored.request_id)? { + if existing != stored { + tracing::warn!( + request_id = %stored.request_id, + "rejected conflicting SSH certificate request during sync" + ); + } + return Ok(false); + } + store.insert_ssh_cert_request(&stored)?; + Ok(true) +} + fn ssh_cert_request_from_stored(stored: StoredSshCertRequest) -> Result { let cert_kind = stored .cert_kind @@ -4826,6 +4850,28 @@ fn stored_from_ssh_certificate(certificate: &SshCertificateRecord) -> StoredSshC } } +fn insert_ssh_certificate_if_not_conflicting( + store: &Store, + certificate: &SshCertificateRecord, +) -> Result { + let stored = stored_from_ssh_certificate(certificate); + if let Some(existing) = store + .list_ssh_certificates()? + .into_iter() + .find(|existing| existing.cert_id == stored.cert_id) + { + if existing != stored { + tracing::warn!( + cert_id = %stored.cert_id, + "rejected conflicting SSH certificate during sync" + ); + } + return Ok(false); + } + store.insert_ssh_certificate(&stored)?; + Ok(true) +} + fn ssh_certificate_from_stored(stored: StoredSshCertificate) -> SshCertificateRecord { SshCertificateRecord { id: SshCertId::new(stored.cert_id), @@ -4847,6 +4893,28 @@ fn stored_from_ssh_revocation(revocation: &SshRevocationEntry) -> StoredSshRevoc } } +fn insert_ssh_revocation_if_not_conflicting( + store: &Store, + revocation: &SshRevocationEntry, +) -> Result { + let stored = stored_from_ssh_revocation(revocation); + if let Some(existing) = store + .list_ssh_revocations()? + .into_iter() + .find(|existing| existing.revocation_id == stored.revocation_id) + { + if existing != stored { + tracing::warn!( + revocation_id = %stored.revocation_id, + "rejected conflicting SSH revocation during sync" + ); + } + return Ok(false); + } + store.insert_ssh_revocation(&stored)?; + Ok(true) +} + fn ssh_revocation_from_stored( stored: StoredSshRevocation, ) -> Result { @@ -4951,6 +5019,70 @@ mod tests { .expect("store auth grant"); } + #[test] + fn ssh_sync_import_rejects_conflicting_records() { + let store = Store::open_memory().expect("open"); + let request = SshCertRequest { + id: SshCertRequestId::new("ssh-cert-request:1"), + requester_node: NodeId::new("node:right"), + public_key: "ssh-ed25519 AAAA".to_owned(), + public_key_fingerprint: "SHA256:one".to_owned(), + cert_kind: SshCertKind::User, + principals: vec!["eric".to_owned()], + requested_validity: None, + renewal_of: None, + reason: Some("original".to_owned()), + status: SshCertRequestStatus::Pending, + created_at: UnixMillis(1), + }; + assert!( + insert_ssh_cert_request_if_not_conflicting(&store, &request).expect("insert request") + ); + let mut conflicting_request = request.clone(); + conflicting_request.reason = Some("conflict".to_owned()); + assert!( + !insert_ssh_cert_request_if_not_conflicting(&store, &conflicting_request) + .expect("reject conflicting request") + ); + assert_eq!( + store + .get_ssh_cert_request(request.id.as_str()) + .expect("get request") + .expect("request") + .reason + .as_deref(), + Some("original") + ); + + let revocation = SshRevocationEntry { + id: geth_types::SshRevocationId::new("ssh-revocation:1"), + kind: SshRevocationKind::KeyId, + target: "old-key".to_owned(), + reason: Some("original".to_owned()), + created_at: UnixMillis(2), + published: true, + }; + assert!( + insert_ssh_revocation_if_not_conflicting(&store, &revocation) + .expect("insert revocation") + ); + let mut conflicting_revocation = revocation.clone(); + conflicting_revocation.target = "other-key".to_owned(); + assert!( + !insert_ssh_revocation_if_not_conflicting(&store, &conflicting_revocation) + .expect("reject conflicting revocation") + ); + assert_eq!( + store + .list_ssh_revocations() + .expect("list revocations") + .first() + .expect("revocation") + .target, + "old-key" + ); + } + #[test] fn sync_watermarks_include_only_authorized_streams() { let store = Store::open_memory().expect("open"); diff --git a/docs/architecture.md b/docs/architecture.md index 3b0735a..7ac358e 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -223,11 +223,13 @@ unsupported and asks for JSONL or the spec source. Revocation lists are not yet full CRDT-replicated resources, but the daemon can already pull cert-flow and revocation metadata from authorized peers over the protected Iroh control ALPN. Manual sync commands and the background live-sync loop share the same capability -checks and cursor state. Local SSH certificate and revocation metadata commands -also accept an optional subject principal for authorization testing: non-owner -subjects must hold `ssh_cert.*` capabilities on `resource:ssh:certs` or -`ssh_revocation.*` capabilities on `resource:ssh:revocations` before requests, -approval/import/read operations, or revocation publish/read/import operations +checks and cursor state. Sync import rejects conflicting records with ids that +already exist locally instead of replacing local metadata. Local SSH certificate +and revocation metadata commands also accept an optional subject principal for +authorization testing: non-owner subjects must hold `ssh_cert.*` capabilities on +`resource:ssh:certs` or `ssh_revocation.*` capabilities on +`resource:ssh:revocations` before requests, approval/import/read operations, or +revocation publish/read/import operations are accepted. The live-sync loop first asks for authorized stream watermarks and skips module pulls whose remote high-water value has not advanced. diff --git a/docs/roadmap.md b/docs/roadmap.md index d9b9fcf..3a3da6d 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -264,8 +264,12 @@ resource-scoped capability decisions. `ssh_revocation.import` for explicit non-owner `--subject` principals. - `[x]` Tests cover denied and granted non-owner local SSH cert request and revocation publish flows. + - `[x]` Sync import rejects conflicting certificate request, certificate, and + revocation records with ids that already exist locally. + - `[x]` Tests verify conflicting SSH cert request and revocation records do + not overwrite local metadata. - `[ ]` Future completion requires all accepted SSH cert/revocation records - to be signed and reducible before replication. + to carry signed provenance and reduce cleanly before replication. ## Phase 3: CAS, KV, And Pubsub @@ -400,7 +404,10 @@ Goal: add authorized stream-oriented management workflows over Iroh. - `[x]` Background live-sync uses the same protected Iroh path and cursor state as manual sync. - `[ ]` Replace pull-only metadata sync with a resource log or CRDT model. - - `[ ]` Conflicting or unsigned records are rejected or quarantined. + - `[x]` Conflicting records with already-known ids are rejected during import + rather than replacing local metadata. + - `[ ]` Unsigned records are rejected or quarantined once signed provenance is + part of the metadata format. - `[x]` OpenSSH KRL import/export. Acceptance criteria: