Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 28 additions & 9 deletions crates/sp42-server/src/action_routes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -273,17 +273,21 @@ async fn execute_session_action(
let rev_ids = payload
.batch_rev_ids
.clone()
.filter(|ids| !ids.is_empty())
.unwrap_or_else(|| vec![payload.rev_id]);
let (first, rest) = rev_ids
.split_first()
.expect("rev_ids is non-empty: the empty fallback yields [payload.rev_id]");
let mut response = execute_patrol(
client,
config,
&PatrolRequest {
rev_id: rev_ids[0],
rev_id: *first,
token: token.clone(),
},
)
.await?;
for rid in rev_ids.iter().skip(1) {
for rid in rest {
response = execute_patrol(
client,
config,
Expand Down Expand Up @@ -981,6 +985,27 @@ fn action_shell_feedback(
feedback
}

/// Validate a Patrol request: reject a present-but-empty batch (which would
/// otherwise index `rev_ids[0]` on an empty Vec and panic in
/// `execute_session_action`) and require the patrol capability. A missing
/// `batch_rev_ids` is fine — it falls back to the single `rev_id`.
fn validate_patrol_request(
payload: &SessionActionExecutionRequest,
capabilities: &DevAuthCapabilityReport,
) -> Result<(), (StatusCode, Json<serde_json::Value>)> {
if payload.batch_rev_ids.as_ref().is_some_and(Vec::is_empty) {
return Err(invalid_payload(
"batch_rev_ids must be non-empty when present",
));
}
if !capabilities.capabilities.moderation.can_patrol {
return Err(forbidden_error(
"The authenticated session does not currently have patrol capability on this wiki.",
));
}
Ok(())
}

pub(crate) fn validate_action_request(
payload: &SessionActionExecutionRequest,
capabilities: &DevAuthCapabilityReport,
Expand All @@ -1006,13 +1031,7 @@ pub(crate) fn validate_action_request(
));
}
}
SessionActionKind::Patrol => {
if !capabilities.capabilities.moderation.can_patrol {
return Err(forbidden_error(
"The authenticated session does not currently have patrol capability on this wiki.",
));
}
}
SessionActionKind::Patrol => validate_patrol_request(payload, capabilities)?,
SessionActionKind::Undo => {
if payload.title.as_deref().is_none_or(str::is_empty) {
return Err(invalid_payload("title is required for undo"));
Expand Down
21 changes: 20 additions & 1 deletion crates/sp42-server/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -408,8 +408,27 @@ async fn handle_socket(socket: WebSocket, wiki_id: String, actor: Option<String>
let mut subscriber = state.coordination.subscribe(&wiki_id).await;
let (mut sender, mut receiver) = socket.split();

let log_wiki_id = wiki_id.clone();
let send_task = tokio::spawn(async move {
while let Ok(envelope) = subscriber.recv().await {
loop {
let envelope = match subscriber.recv().await {
Ok(envelope) => envelope,
// A slow client that fell more than the room's buffer behind
// dropped `skipped` messages. Resync and keep serving live
// updates rather than killing the fan-out task (which would
// silently stop delivering to an otherwise-open socket).
Err(tokio::sync::broadcast::error::RecvError::Lagged(skipped)) => {
warn!(
wiki_id = %log_wiki_id,
client_id,
skipped,
"coordination subscriber lagged; resyncing to live"
);
continue;
}
// Sender dropped (room evicted): nothing more will arrive.
Err(tokio::sync::broadcast::error::RecvError::Closed) => break,
};
if envelope.sender_id == client_id {
continue;
}
Expand Down
35 changes: 35 additions & 0 deletions crates/sp42-server/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3222,6 +3222,41 @@ fn validate_accepts_inline_edit_with_node_locator() {
assert!(crate::action_routes::validate_action_request(&payload, &report).is_ok());
}

#[test]
fn validate_rejects_patrol_with_an_empty_batch() {
// A present-but-empty batch_rev_ids would index rev_ids[0] on an empty
// Vec and panic in execute_session_action; validation must reject it.
let report = sp42_core::DevAuthCapabilityReport {
checked: true,
wiki_id: "frwiki".to_string(),
capabilities: sp42_core::DevAuthDerivedCapabilities {
moderation: sp42_core::DevAuthModerationCapabilities {
can_patrol: true,
..Default::default()
},
..Default::default()
},
..Default::default()
};
let mut payload = inline_edit_request(None, None, None);
payload.kind = sp42_core::SessionActionKind::Patrol;
payload.batch_rev_ids = Some(vec![]);

let (status, body) = crate::action_routes::validate_action_request(&payload, &report)
.expect_err("an empty patrol batch must be rejected");
assert_eq!(status, axum::http::StatusCode::BAD_REQUEST);
assert!(
body.0["error"]
.as_str()
.expect("error body should carry a message")
.contains("batch_rev_ids")
);

// A missing batch (None) still falls back to the single rev_id.
payload.batch_rev_ids = None;
assert!(crate::action_routes::validate_action_request(&payload, &report).is_ok());
}

#[test]
fn validate_rejects_inline_edit_without_selected_text_or_locator() {
let payload = inline_edit_request(None, None, Some("x".to_string()));
Expand Down
Loading