From 8ea1f7daee68c5224dd5ff596eb32444c34bceaf Mon Sep 17 00:00:00 2001 From: atlas Date: Sun, 20 Sep 2026 15:36:08 +0200 Subject: [PATCH] hive-c0re: assert the scheduling surface permits an unrelated requester MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous commit deleted `schedule_authorized` and with it three denial tests, leaving the permit behaviour it introduced unasserted: nothing in the suite would notice a subtree check creeping back into the scheduling handlers. These four pin the inverse of the decision the deleted predicate used to make. They sit at the handler level because that is the lowest layer where the decision still exists to be observed — the predicate, its pure `_in` form and the wrapper are gone, so there is no function left whose return value *is* the decision, and the layer above (`dispatch`) reaches these verbs through `require_group("scheduling")`, a different gate that is still present and not the one under test. `cancel` and `edit` assert the row afterwards rather than stopping at `Response::Ok`: a check that silently declines to act while still answering `Ok` is the regression a response-code assertion misses. `list_schedules` is the structurally different case — it never rejected, it filtered per row, so its assertion is that another owner's row is present at all. `fire` asserts the response only; the fan-out legitimately finds no live container under test and `fire_now` reports that inside `Ok(report)`, so the response code is the only honest signal there. Verified by mutation, not by inspection: reintroducing an ancestry walk into the four handlers flips all four tests to failing (the three mutating verbs on the refusal, `list_schedules` on an empty list) and leaves the two pre-existing ghost-filter tests untouched. These four cover the complete set of gate points this branch removes. The roster listing that used to be the fifth is no longer one: `main` removed `list_containers`, `Request::ListDescendants` and the handler behind them outright, so there is nothing left there to assert. Refs #4472 --- hive-c0re/src/socket_server/schedules.rs | 132 +++++++++++++++++++++++ 1 file changed, 132 insertions(+) diff --git a/hive-c0re/src/socket_server/schedules.rs b/hive-c0re/src/socket_server/schedules.rs index 15e52ff8..d41d2dfe 100644 --- a/hive-c0re/src/socket_server/schedules.rs +++ b/hive-c0re/src/socket_server/schedules.rs @@ -353,4 +353,136 @@ mod tests { filter_ghost_schedule_targets(&mut schedules, &live); assert!(schedules[0].targets.is_empty()); } + + // ----------------------------------------------------------------------- + // The scheduling surface permits an unrelated requester + // + // These pin the *decision* the deleted `schedule_authorized` used to make, + // inverted: a requester with no relation to a schedule's owner reaches the + // handler's effect instead of a refusal. They live at the handler level + // because the predicate, its pure `_in` form and all three wrappers are + // gone — the handler is the lowest layer where the decision still exists + // to be observed. + // + // `OWNER` and `STRANGER` are names no hive has, so no `topology.json` on + // any machine running these tests can relate them. Any ancestry check + // reintroduced between the two — disk-reading or pure — therefore answers + // "unrelated", and every assertion below flips from the handler's effect + // to a refusal. + // ----------------------------------------------------------------------- + + /// Owner of the schedule under test. + const OWNER: &str = "schedule-owner-with-no-relations"; + /// Requester acting on it: not the owner, not the operator, and not + /// anybody's ancestor or descendant. + const STRANGER: &str = "requester-with-no-relations"; + + /// A real `Coordinator` over a throwaway sqlite dir. The schedule + /// handlers take `&Arc`, so there is no lighter way in; + /// `open` touches nothing outside the db path it is handed. + fn coordinator() -> (tempfile::TempDir, Arc) { + let dir = tempfile::tempdir().expect("tempdir"); + let coord = Coordinator::open( + &dir.path().join("broker.sqlite"), + crate::coordinator::HiveEnv::default(), + crate::hive_stats::PriceTable::new(), + 1, + ) + .expect("open coordinator"); + (dir, Arc::new(coord)) + } + + /// Insert a schedule owned by [`OWNER`], returning its id. + fn submit_owned_by_other(coord: &Arc) -> i64 { + coord + .scheduled_prompts + .submit(&crate::scheduled_prompts::NewSchedule { + owner: OWNER.to_owned(), + targets: vec!["iris".to_owned()], + body: "ping".to_owned(), + first_fire_at_unix: 100, + interval_seconds: None, + description: None, + source: crate::scheduled_prompts::ScheduleSource::Operator, + }) + .expect("submit") + } + + #[test] + fn cancel_schedule_permits_unrelated_requester() { + let (_dir, coord) = coordinator(); + let id = submit_owned_by_other(&coord); + let resp = handle_cancel_schedule(&coord, STRANGER, id, None); + assert!( + matches!(resp, Response::Ok), + "unrelated requester must be allowed to cancel; got {resp:?}" + ); + // Ok alone could be a refusal that forgot to say so — check the row. + let row = coord + .scheduled_prompts + .get(id) + .expect("read back") + .expect("row present"); + assert!( + row.cancelled_at_unix.is_some(), + "cancel must have taken effect, not just answered Ok" + ); + } + + #[test] + fn edit_schedule_permits_unrelated_requester() { + let (_dir, coord) = coordinator(); + let id = submit_owned_by_other(&coord); + let resp = handle_edit_schedule( + &coord, + STRANGER, + id, + EditSchedulePatch { + body: Some("edited by a stranger".to_owned()), + description: None, + interval_seconds: None, + next_fire_at_unix: None, + targets_add: None, + targets_remove: None, + }, + ); + assert!( + matches!(resp, Response::Ok), + "unrelated requester must be allowed to edit; got {resp:?}" + ); + let row = coord + .scheduled_prompts + .get(id) + .expect("read back") + .expect("row present"); + assert_eq!(row.body, "edited by a stranger"); + } + + #[tokio::test] + async fn fire_schedule_now_permits_unrelated_requester() { + let (_dir, coord) = coordinator(); + let id = submit_owned_by_other(&coord); + let resp = handle_fire_schedule_now(&coord, STRANGER, id).await; + // The fan-out itself is allowed to find no live target (this test has + // no containers) — that is reported inside the report, not as an Err. + assert!( + matches!(resp, Response::Ok), + "unrelated requester must be allowed to fire; got {resp:?}" + ); + } + + #[test] + fn list_schedules_shows_a_schedule_owned_by_someone_else() { + let (_dir, coord) = coordinator(); + let id = submit_owned_by_other(&coord); + let Response::Schedules { schedules } = handle_list_schedules(&coord, STRANGER) else { + panic!("expected a Schedules response"); + }; + // The deleted filter dropped every row the requester was unrelated to, + // which for this requester is all of them. + assert!( + schedules.iter().any(|s| s.id == id && s.owner == OWNER), + "unrelated requester must see another agent's schedule; got {schedules:?}" + ); + } }