diff options
| author | tv <tv@krebsco.de> | 2026-11-08 18:20:13 +0000 |
|---|---|---|
| committer | tv <tv@krebsco.de> | 2026-11-08 19:29:15 +0100 |
| commit | ff9e7c244c270853a8a8f58ee950859866358e1d (patch) | |
| tree | 78070114c1bc0d48814d1ef260d1c8c7a70a92c0 /src/access | |
| parent | 96ba15cf1a6f8725f70f429695a5a9661f706eb0 (diff) | |
remake: aborts written as the user who asks; no abort by time
Review 2, N3: 1150 let any member write an abort decision once a remake
had expired a grace period ago, judged by the change's own time, the
writer's word. A member could race the coordinator's legitimate commit
with an abort dated as it liked; both taken, Automerge shows
one, and nodes that acted on the commit split from those that didn't.
The two writers of such aborts are operator requests whose endpoints
already need write on _continuum/remakes: aborting an expired remake
(DELETE /admin/remakes/{id}) and superseding one (POST
/admin/remakes?force=supersede). Both now commit as the user who asks
(the proposal and its vote with them, for a supersede), which the rules
take as one who may write the decision. The clause judging an abort by
its time goes, and with it the time the rules carried. An expired
remake is still aborted through any member, by an operator.
Diffstat (limited to 'src/access')
| -rw-r--r-- | src/access/rules.rs | 170 | ||||
| -rw-r--r-- | src/access/screen.rs | 1 |
2 files changed, 11 insertions, 160 deletions
diff --git a/src/access/rules.rs b/src/access/rules.rs index b5dcf3a..ed9ee4b 100644 --- a/src/access/rules.rs +++ b/src/access/rules.rs @@ -56,8 +56,6 @@ pub(crate) struct Change<'a> { pub(crate) paths: &'a BTreeSet<Path>, /// Its operations, where each is and what it does there (`inspect::Inspector::ops`). pub(crate) ops: &'a [Op], - /// When it was made (ms), as its writer says. - pub(crate) time: u64, } /// Why a change is refused. @@ -214,12 +212,10 @@ pub(crate) fn judge(doc: &Document, epoch: Epoch, change: &Change<'_>) -> Result // Its coordinator's; or the record of the remake that made the epoch, // carried into it whole (its proposal written with it) by the coordinator // its snapshot names (`remake::switch`), where there is none. - // Also by one who may write it; and an abort by any member, once the - // remake expired a grace period ago (`aborts_expired`). + // Also by one who may write it (an operator aborting an expired remake, + // or superseding it, `remake::api`). Some("decision") => match coordinator(&at, id) { - Some(coordinator) => { - coordinator == node || user_may(path) || aborts_expired(&at, id, change) - } + Some(coordinator) => coordinator == node || user_may(path), None if user_may(path) => true, None => { change @@ -306,7 +302,6 @@ pub(crate) fn judge_held( user: user.as_deref(), paths: &paths, ops: &ops, - time: u64::try_from(change.timestamp()).unwrap_or(0), }, ); Some(match judged { @@ -455,38 +450,6 @@ fn made_the_epoch(at: &At<'_>, epoch: Epoch, id: &str, node: &str) -> bool { == Some(node) } -/// Whether `change` aborts remake `id`, undecided at `at`, as any member may: once it -/// expired a grace period ago (`remake::proposal::abortable_at`, its coordinator's grace -/// period), by the change's time. That is its writer's word: a member could abort a -/// remake early, as it could ask its coordinator to; never commit one. -fn aborts_expired(at: &At<'_>, id: &str, change: &Change<'_>) -> bool { - if store::read_path(at, &meta::path(&[meta::REMAKES, id, "decision"])) - .ok() - .flatten() - .is_some() - { - return false; - } - let outcome = meta::path(&[meta::REMAKES, id, "decision", "outcome"]); - let mut outcomes = change.ops.iter().filter(|op| op.path == outcome).peekable(); - if outcomes.peek().is_none() - || !outcomes.all(|op| matches!(&op.action, Action::PutString(value) if value == "aborted")) - { - return false; - } - let Some(proposal) = store::read_path(at, &meta::path(&[meta::REMAKES, id, "proposal"])) - .ok() - .flatten() - else { - return false; - }; - let number = |key: &str| proposal.get(key).and_then(serde_json::Value::as_u64); - let (Some(settle_until), Some(grace)) = (number("settleUntil"), number("graceMs")) else { - return false; - }; - change.time >= settle_until.saturating_add(grace.saturating_mul(2)) -} - /// Whether `owner` has a key left after `ops`, its keys at `at` as they change them. fn keys_left(at: &At<'_>, owner: &str, ops: &[Op]) -> bool { let keys = meta::path(&[meta::USERS, owner, "keys"]); @@ -593,7 +556,6 @@ mod tests { user, paths: &touched, ops: &[], - time: 0, }, ) } @@ -678,7 +640,6 @@ mod tests { user: None, paths: &touched, ops: &[], - time: 0, }, ) }; @@ -766,130 +727,26 @@ mod tests { user: None, paths: &touched, ops: &[], - time: 0, }, ); assert!(earlier.is_ok()); } - /// A remake's decision: its coordinator's, one who may write it; an abort, any - /// member's, once the remake expired a grace period ago, by the change's time. + /// A remake's decision: its coordinator's, or one who may write it; no other + /// member's, whatever its time. #[test] - fn expired_remakes_aborted_by_any_member() { + fn decisions_by_the_coordinator_or_who_may() { let mut doc = swarm(true); write( &mut doc, - "_continuum/remakes/r4/proposal", - json!({"coordinator": "a", "settleUntil": 1_000, "graceMs": 100}), - ); - write( - &mut doc, "_continuum/users/carol", registry::entry("a", 2, &[(path("_continuum"), &[Right::Write][..])]), ); - let decide = |user: Option<&str>, outcome: &str, time: u64| { - let ops = vec![ - Op { - path: path("_continuum/remakes/r4/decision"), - action: Action::MakeMap, - }, - Op { - path: path("_continuum/remakes/r4/decision/outcome"), - action: Action::PutString(outcome.to_owned()), - }, - Op { - path: path("_continuum/remakes/r4/decision/decidedAt"), - action: Action::Other, - }, - ]; - let deps = doc.get_heads(); - let touched: BTreeSet<Path> = ops.iter().map(|op| op.path.clone()).collect(); - judge( - &doc, - Epoch::GENESIS, - &Change { - hash: ChangeHash([9; 32]), - deps: &deps, - seen: &deps, - committer: Some("b"), - user, - paths: &touched, - ops: &ops, - time, - }, - ) - }; - // Expired at 1100; abortable by others a grace period later. - assert!(refused(decide(None, "aborted", 1_199))); - assert!(decide(None, "aborted", 1_200).is_ok()); - assert!(refused(decide(None, "committed", 5_000))); - // One who may write protocol data, any decision, any time. - assert!(decide(Some("carol"), "committed", 0).is_ok()); - } - - /// A user's own keys: its own to change, whatever else it may do, one left at least; - /// another's, or the rest of its own entry, not. - #[test] - fn users_change_their_own_keys() { - let mut doc = swarm(true); - let judged_ops = |doc: &Document, user: Option<&str>, ops: Vec<Op>| { - let deps = doc.get_heads(); - let touched: BTreeSet<Path> = ops.iter().map(|op| op.path.clone()).collect(); - judge( - doc, - Epoch::GENESIS, - &Change { - hash: ChangeHash([9; 32]), - deps: &deps, - seen: &deps, - committer: Some("a"), - user, - paths: &touched, - ops: &ops, - time: 0, - }, - ) - }; - let op = |at: &str, action: Action| Op { - path: path(at), - action, - }; - let put = |at: &str| op(at, Action::PutString("ssh-ed25519 AAAA".to_owned())); - let bobs = "_continuum/users/bob/keys/laptop"; - assert!(judged_ops(&doc, Some("bob"), vec![put(bobs)]).is_ok()); - assert!(refused(judged_ops( - &doc, - Some("bob"), - vec![put("_continuum/users/alice/keys/laptop")] - ))); - assert!(refused(judged_ops( - &doc, - Some("bob"), - vec![op("_continuum/users/bob/caps", Action::MakeMap)] - ))); - assert!(refused(judged_ops(&doc, None, vec![put(bobs)]))); - // With one key: another added, one of two removed; not the last. - write(&mut doc, bobs, json!("ssh-ed25519 AAAA")); - let phone = "_continuum/users/bob/keys/phone"; - assert!(judged_ops(&doc, Some("bob"), vec![put(phone)]).is_ok()); - assert!(refused(judged_ops( - &doc, - Some("bob"), - vec![op(bobs, Action::Delete)] - ))); - assert!( - judged_ops( - &doc, - Some("bob"), - vec![put(phone), op(bobs, Action::Delete)] - ) - .is_ok() - ); - assert!(refused(judged_ops( - &doc, - Some("bob"), - vec![op("_continuum/users/bob/keys", Action::Delete)] - ))); + let decision = "_continuum/remakes/r1/decision"; + assert!(judged(&doc, "a", None, &[decision]).is_ok()); + assert!(refused(judged(&doc, "b", None, &[decision]))); + assert!(refused(judged(&doc, "b", Some("bob"), &[decision]))); + assert!(judged(&doc, "b", Some("carol"), &[decision]).is_ok()); } /// Under a strict policy, a user's own entry is its own to write (what it may sign @@ -973,7 +830,6 @@ mod tests { user: Some("alice"), paths: &touched, ops: &[], - time: 0, }, ) }; @@ -1037,7 +893,6 @@ mod tests { user: Some("alice"), paths: &touched, ops: &ops, - time: 0, }, ) }; @@ -1128,7 +983,6 @@ mod tests { user: Some("alice"), paths: &touched, ops: &ops, - time: 0, }, ) }; @@ -1206,7 +1060,6 @@ mod tests { user: Some("bob"), paths: &touched, ops: &ops, - time: 0, }, ) }; @@ -1261,7 +1114,6 @@ mod tests { user: None, paths: &touched, ops: &[], - time: 0, }, ) }; diff --git a/src/access/screen.rs b/src/access/screen.rs index 5584b2b..d7c9389 100644 --- a/src/access/screen.rs +++ b/src/access/screen.rs @@ -442,7 +442,6 @@ pub(crate) fn apply( user: user.as_deref(), paths: &paths[index], ops: &ops[index], - time: u64::try_from(change.timestamp()).unwrap_or(0), }, ); match judged { |
