You are a constitutional council ranking individual git commits for ownership allocation. Compare these two commits. Decide which contributed more lasting value to the project. Judge substance, not spectacle: - Prefer correct, lasting design and real bugfixes over churn, formatting, renames, or generated noise. - Prefer clarity and necessity over sheer line count. A small precise change can beat a large diffuse one. - Do not favor a side merely because its patch is longer or noisier. - Weight what the change does for the project, not the contributor's name. Return ONLY a JSON object: {"winner": "A" or "B", "ratio": "N:M", "explanation": "..."} The explanation must cite concrete differences in the patches (1-3 sentences). Side A — contributor: tommy-mor Side A — commit message: [880eb778] Harden auth: fail-closed votes, mock OAuth gate, Secure cookies. Also show the current alias in the top nav and pin durable by rev. Co-authored-by: Cursor Side A — unified diff (full patch): diff --git a/AGENTS.md b/AGENTS.md index babb889d6fbfb1fa7176c9e6b7544ae17b61dd2e..6e0fd8ebb65d665c9c1438e3275971d62b98fd95 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -10,11 +10,11 @@ Single Rust web app **`sorter2-server`**: pairwise voting, rank-centrality ranki - **Bootstrap script**: `./scripts/cursor-env-install.sh` (also run via `.cursor/environment.json` on Cloud Agent boot) installs Playwright Chromium, Babashka, bbin, `clj-paren-repair`, and warms the RocksDB build. - **Rust 1.88+** is required (`rust-toolchain.toml`). The Cloud Dockerfile and `cursor-env-install.sh` install **rustup** 1.88.0 first so `cargo` works while Playwright/Clojure bootstrap continues. Do not rely on `/usr/local/cargo` (often missing or stale). -- **RocksDB / `durable`**: Ubuntu’s default `c++` is often **clang** without libc++ headers. Set **`CXX=g++`** and **`RUSTFLAGS="-C linker=g++"`** (or `CC=gcc`) before `cargo build` / `cargo test` — both are set in the bootstrap script and `.cursor/environment.json`. +- **RocksDB / `durable`**: `durable` is an external git dependency (`tommy-mor/durable`, pinned by rev in `server/Cargo.toml`). Ubuntu’s default `c++` is often **clang** without libc++ headers. Set **`CXX=g++`** and **`RUSTFLAGS="-C linker=g++"`** (or `CC=gcc`) before `cargo build` / `cargo test` — both are set in the bootstrap script and `.cursor/environment.json`. - **System packages** for builds: `build-essential`, `g++`, `clang`, `libclang-dev`, `pkg-config`, `libssl-dev`, `openjdk-21-jre-headless` (for `reqwest` / OpenSSL, `librocksdb-sys`, `zstd-sys` / bindgen, and **bbin** / Clojure JVM). The bootstrap sets **`JAVA_HOME`** when Java is present. - **Clojure CLI 1.12.0.1530** (used in CI): install from https://clojure.org/guides/install_clojure — needed for `./scripts/clj-test.sh` / Kaocha tests. - **Babashka / bbin / clj-paren-repair**: installed by `cursor-env-install.sh` into `~/.local/bin` (bb tasks in `bb.edn`, delimiter repair for Clojure edits). -- **Playwright** (Spel browser tests in `test/vote_compare.clj`): Chromium via `clojure -M -e "(com.microsoft.playwright.CLI/main ...)"` — run once after clone or use the bootstrap script. +- **Playwright** (Spel browser tests in `test/vote_compare.clj` / `test/auth_login.clj`): Chromium via `clojure -M -e "(com.microsoft.playwright.CLI/main ...)"` — run once after clone or use the bootstrap script. ### Commands (see also `TEST.sh`) @@ -34,13 +34,17 @@ Environment variables (defaults in `server/src/state.rs`): - `PORT` — default `8080` - `SORTER2_DATA_DIR` — default `./data` (created on startup) - `SORTER2_EVENT_LOG` — default `{data_dir}/events.jsonl` +- `SORTER2_BASE_URL` — public origin (also drives Secure cookies when `https://`) +- `GITHUB_CLIENT_ID` / `GITHUB_CLIENT_SECRET` — GitHub OAuth (optional; login disabled if unset) +- `SORTER2_ALLOW_MOCK_OAUTH=1` — allow `mock_user` on `/auth/github` (tests only) Health check: `GET /healthz` → `ok`. -Core UI flow: `POST /ui` with form field `__rpc__` (JSON). Example vote: +Core UI flow: `POST /ui` with form field `__rpc__` (JSON). Votes require a session cookie (sign in via `/login`). Example vote: ```bash curl -sf -X POST http://127.0.0.1:8080/ui \ + --cookie "sorter2_session=..." \ --data-urlencode '__rpc__={"action":"record_vote","a":"alpha","b":"beta","ratio_left":2,"ratio_right":1}' ``` diff --git a/Cargo.lock b/Cargo.lock index aa02997ad85777195f135bfd9456bcee0fc9a590..1f8690f3e486d099577a32c2ece48caf57ea7160 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -414,7 +414,7 @@ dependencies = [ [[package]] name = "durable" version = "0.2.0" -source = "git+https://github.com/tommy-mor/durable.git?branch=main#a6c14eaa809693140eea0c22b07ef24d8e74adaf" +source = "git+https://github.com/tommy-mor/durable.git?rev=a6c14eaa809693140eea0c22b07ef24d8e74adaf#a6c14eaa809693140eea0c22b07ef24d8e74adaf" dependencies = [ "ciborium", "durable-derive", @@ -426,7 +426,7 @@ dependencies = [ [[package]] name = "durable-derive" version = "0.2.0" -source = "git+https://github.com/tommy-mor/durable.git?branch=main#a6c14eaa809693140eea0c22b07ef24d8e74adaf" +source = "git+https://github.com/tommy-mor/durable.git?rev=a6c14eaa809693140eea0c22b07ef24d8e74adaf#a6c14eaa809693140eea0c22b07ef24d8e74adaf" dependencies = [ "proc-macro2", "quote", diff --git a/server/Cargo.toml b/server/Cargo.toml index dfa39beddecfa37dcdeaa602cb30f4b547528fbb..bd88687fb0ba47d68f2c08eb5e11d0e08b7c4398 100644 --- a/server/Cargo.toml +++ b/server/Cargo.toml @@ -25,7 +25,7 @@ futures-util = { version = "0.3", default-features = false, features = ["std"] } rand = "0.8" urlencoding = "2" url = "2" -durable = { git = "https://github.com/tommy-mor/durable.git", branch = "main" } +durable = { git = "https://github.com/tommy-mor/durable.git", rev = "a6c14eaa809693140eea0c22b07ef24d8e74adaf" } [dev-dependencies] reqwest = { version = "0.12", features = ["json"] } diff --git a/server/src/api/ui_html.rs b/server/src/api/ui_html.rs index b86581b1f337650564274254d840e8a75b49524d..9da62ffbed07eb28729aa3160bf33b07ce0d7945 100644 --- a/server/src/api/ui_html.rs +++ b/server/src/api/ui_html.rs @@ -71,10 +71,16 @@ pub async fn post_ui_html( return resp; } let parent = parent_from_scope(&scope); - let actor = resolve_vote_actor( + let actor = match resolve_vote_actor( state.projection_store.db(), session_id_from_jar(&jar).as_deref(), - ); + ) { + Ok(actor) => actor, + Err(_) => { + return vote_auth_redirect(&state, &jar) + .unwrap_or_else(|| login_redirect_js().into_response()); + } + }; if let Err(e) = state .record_vote(&parent, &a, &b, ratio_left, ratio_right, &actor) .await diff --git a/server/src/auth/config.rs b/server/src/auth/config.rs index a1f042c655bf3e5234eeb87a7d889f64592807fb..a5976af9a52ea207b35ae87bd1fe927c47a477ca 100644 --- a/server/src/auth/config.rs +++ b/server/src/auth/config.rs @@ -1,9 +1,42 @@ pub const AUTH_RETURN_COOKIE: &str = "sorter2_auth_return"; +/// Allow `mock_user` on `/auth/github` (test harness only). +pub fn mock_oauth_allowed() -> bool { + matches!( + std::env::var("SORTER2_ALLOW_MOCK_OAUTH").as_deref(), + Ok("1") | Ok("true") | Ok("TRUE") + ) +} + +/// Set the Secure flag on auth cookies when serving over HTTPS. +pub fn cookies_secure() -> bool { + std::env::var("SORTER2_BASE_URL") + .map(|u| u.starts_with("https://")) + .unwrap_or(false) +} + pub fn sanitize_return_to(raw: &str) -> String { let s = raw.trim(); - if s.is_empty() || !s.starts_with('/') || s.starts_with("//") { + if s.is_empty() || !s.starts_with('/') || s.starts_with("//") || s.starts_with("/\\") { + return "/".to_string(); + } + // Reject scheme-relative and protocol-smuggling forms. + if s.contains("://") || s.contains('\\') { return "/".to_string(); } s.to_string() } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn sanitize_return_to_blocks_open_redirects() { + assert_eq!(sanitize_return_to(""), "/"); + assert_eq!(sanitize_return_to("//evil.com"), "/"); + assert_eq!(sanitize_return_to("/\\evil.com"), "/"); + assert_eq!(sanitize_return_to("https://evil.com"), "/"); + assert_eq!(sanitize_return_to("/vote?parent=x"), "/vote?parent=x"); + } +} diff --git a/server/src/auth/mod.rs b/server/src/auth/mod.rs index 5ed535ba199fa736f0048c32623b14c3b1e5de2d..d4a85ef52c15dc35148e4c743f0d646cbbdb056d 100644 --- a/server/src/auth/mod.rs +++ b/server/src/auth/mod.rs @@ -26,7 +26,7 @@ use crate::{ ui_action::UI_RPC_FIELD, }; -pub use session::{resolve_vote_actor, session_id_from_jar, VoteActor}; +pub use session::{nav_pseudonym, resolve_vote_actor, session_id_from_jar, VoteActor}; pub fn base_url_from_env(port: u16) -> String { std::env::var("SORTER2_BASE_URL") @@ -168,6 +168,10 @@ pub async fn login_page( "login · sorter2", login_body(session.as_ref(), &aliases, &providers), state.views.get_views("/login"), + session + .as_ref() + .filter(|s| !s.pseudonym.trim().is_empty()) + .map(|s| s.pseudonym.as_str()), ); (jar, Html(markup.into_string())).into_response() } @@ -222,6 +226,7 @@ pub async fn alias_page( "choose alias · sorter2", body, state.views.get_views("/login/alias"), + None, ) .into_string(), ) @@ -237,7 +242,12 @@ pub async fn github_start( .ok_or(StatusCode::SERVICE_UNAVAILABLE)?; let return_to = return_from_query_or_jar(&jar, query.return_to.as_deref()); let state_token = session::new_oauth_state(); - let url = oauth::authorize_url(&cfg, &state_token, query.mock_user.as_deref()); + let mock_user = if config::mock_oauth_allowed() { + query.mock_user.as_deref() + } else { + None + }; + let url = oauth::authorize_url(&cfg, &state_token, mock_user); let jar = jar .add(session::oauth_state_cookie_value(&state_token)) .add(session::auth_return_cookie_value(&return_to)); diff --git a/server/src/auth/session.rs b/server/src/auth/session.rs index 09659240b9455c6fca12db5652e1d31cf8c2acfc..41df030ded3abcafc0ab3887ab769adf103f9aa0 100644 --- a/server/src/auth/session.rs +++ b/server/src/auth/session.rs @@ -5,7 +5,7 @@ use durable::{Db, Durability}; use rand::Rng; use crate::{ - auth::config::AUTH_RETURN_COOKIE, + auth::config::{self, AUTH_RETURN_COOKIE}, fetch::now_ms, identity::{DEFAULT_ACTOR_UUID, DEFAULT_PSEUDONYM}, storage_dto::{SessionDataV1, SESSION_DATA_VERSION}, @@ -37,6 +37,7 @@ pub struct VoteActor { } impl VoteActor { + /// Test / bench helper: seed votes as the default pseudonym without a session. pub fn anon() -> Self { Self { pseudonym: DEFAULT_PSEUDONYM.to_string(), @@ -70,20 +71,51 @@ fn hex_encode(bytes: &[u8]) -> String { bytes.iter().map(|b| format!("{b:02x}")).collect() } -pub fn resolve_vote_actor(db: &Db, session_id: Option<&str>) -> VoteActor { - let Some(session_id) = session_id else { - return VoteActor::anon(); - }; - let Ok(Some(session)) = load_session(db, session_id) else { - return VoteActor::anon(); - }; - if session.expires_at <= now_ms() { - return VoteActor::anon(); +fn build_cookie(name: &'static str, value: String) -> Cookie<'static> { + let mut builder = Cookie::build((name, value)) + .http_only(true) + .same_site(SameSite::Lax) + .path("/"); + if config::cookies_secure() { + builder = builder.secure(true); + } + builder.build() +} + +fn clear_cookie(name: &'static str) -> Cookie<'static> { + let mut builder = Cookie::build((name, "")) + .http_only(true) + .same_site(SameSite::Lax) + .path("/") + .removal(); + if config::cookies_secure() { + builder = builder.secure(true); + } + builder.build() +} + +/// Resolve the vote actor from a live session. Fail-closed: never falls back to anon. +pub fn resolve_vote_actor(db: &Db, session_id: Option<&str>) -> Result { + let session_id = session_id.ok_or("sign in to vote")?; + let session = load_valid_session(db, session_id).ok_or("session expired")?; + if !session_has_pseudonym(&session) { + return Err("choose an alias first"); } let trust_weight = user_trust_weight(db, &session.uuid).unwrap_or(1.0); - VoteActor { + Ok(VoteActor { pseudonym: session.current_pseudonym, trust_weight, + }) +} + +/// Display name for the top nav, if any session is active. +pub fn nav_pseudonym(db: &Db, jar: &CookieJar) -> Option { + let session_id = session_id_from_jar(jar)?; + let session = load_valid_session(db, &session_id)?; + if session_has_pseudonym(&session) { + Some(session.current_pseudonym) + } else { + None } } @@ -151,54 +183,27 @@ pub fn destroy_session(db: &Db, session_id: &str) -> Result<(), String> { } pub fn session_cookie_value(session_id: &str) -> Cookie<'static> { - Cookie::build((SESSION_COOKIE, session_id.to_string())) - .http_only(true) - .same_site(SameSite::Lax) - .path("/") - .build() + build_cookie(SESSION_COOKIE, session_id.to_string()) } pub fn clear_session_cookie() -> Cookie<'static> { - Cookie::build((SESSION_COOKIE, "")) - .http_only(true) - .same_site(SameSite::Lax) - .path("/") - .removal() - .build() + clear_cookie(SESSION_COOKIE) } pub fn oauth_state_cookie_value(state: &str) -> Cookie<'static> { - Cookie::build((OAUTH_STATE_COOKIE, state.to_string())) - .http_only(true) - .same_site(SameSite::Lax) - .path("/") - .build() + build_cookie(OAUTH_STATE_COOKIE, state.to_string()) } pub fn clear_oauth_state_cookie() -> Cookie<'static> { - Cookie::build((OAUTH_STATE_COOKIE, "")) - .http_only(true) - .same_site(SameSite::Lax) - .path("/") - .removal() - .build() + clear_cookie(OAUTH_STATE_COOKIE) } pub fn auth_return_cookie_value(return_to: &str) -> Cookie<'static> { - Cookie::build((AUTH_RETURN_COOKIE, return_to.to_string())) - .http_only(true) - .same_site(SameSite::Lax) - .path("/") - .build() + build_cookie(AUTH_RETURN_COOKIE, return_to.to_string()) } pub fn clear_auth_return_cookie() -> Cookie<'static> { - Cookie::build((AUTH_RETURN_COOKIE, "")) - .http_only(true) - .same_site(SameSite::Lax) - .path("/") - .removal() - .build() + clear_cookie(AUTH_RETURN_COOKIE) } pub fn auth_return_from_jar(jar: &CookieJar) -> Option { @@ -213,28 +218,35 @@ pub fn oauth_state_from_jar(jar: &CookieJar) -> Option { jar.get(OAUTH_STATE_COOKIE).map(|c| c.value().to_string()) } -pub fn actor_uuid_for_vote(db: &Db, session_id: Option<&str>) -> String { - let Some(session_id) = session_id else { - return DEFAULT_ACTOR_UUID.to_string(); - }; - load_session(db, session_id) - .ok() - .flatten() - .filter(|s| s.expires_at > now_ms()) - .map(|s| s.uuid) - .unwrap_or_else(|| DEFAULT_ACTOR_UUID.to_string()) -} - #[cfg(test)] mod tests { use super::*; #[test] - fn missing_session_falls_back_to_anon() { + fn missing_session_is_error() { + let dir = tempfile::tempdir().unwrap(); + let db = Db::open(dir.path()).unwrap(); + assert_eq!(resolve_vote_actor(&db, None).unwrap_err(), "sign in to vote"); + } + + #[test] + fn session_without_pseudonym_is_error() { + let dir = tempfile::tempdir().unwrap(); + let db = Db::open(dir.path()).unwrap(); + let (id, _) = create_session(&db, DEFAULT_ACTOR_UUID, "").unwrap(); + assert_eq!( + resolve_vote_actor(&db, Some(&id)).unwrap_err(), + "choose an alias first" + ); + } + + #[test] + fn session_with_pseudonym_resolves() { let dir = tempfile::tempdir().unwrap(); let db = Db::open(dir.path()).unwrap(); - let actor = resolve_vote_actor(&db, None); - assert_eq!(actor.pseudonym, DEFAULT_PSEUDONYM); + let (id, _) = create_session(&db, DEFAULT_ACTOR_UUID, "alice").unwrap(); + let actor = resolve_vote_actor(&db, Some(&id)).unwrap(); + assert_eq!(actor.pseudonym, "alice"); assert_eq!(actor.trust_weight, 1.0); } } diff --git a/server/src/html/mod.rs b/server/src/html/mod.rs index 46d18b87f1313bf0aeb29955d18f291961057509..3cc3d7bdf55b5cb5d009600f4ade1fcd201a410b 100644 --- a/server/src/html/mod.rs +++ b/server/src/html/mod.rs @@ -4,11 +4,13 @@ use axum::{ http::{header, StatusCode, Uri}, response::{IntoResponse, Response}, }; +use axum_extra::extract::cookie::CookieJar; use maud::{html, Markup, DOCTYPE}; use std::collections::HashSet; use crate::{ + auth::nav_pseudonym, fetch::html::entity_section, form_template::template_json_compact, path_types::ItemId, @@ -126,7 +128,7 @@ pub fn now_ms() -> i64 { t.as_millis() as i64 } -pub(crate) fn layout(title: &str, body: Markup, views: u64) -> Markup { +pub(crate) fn layout(title: &str, body: Markup, views: u64, nav_user: Option<&str>) -> Markup { let ver = asset_version(); let css_href = format!("/static/sorter.css?v={ver}"); let js_src = format!("/static/sorter_ui.js?v={ver}"); @@ -145,7 +147,15 @@ pub(crate) fn layout(title: &str, body: Markup, views: u64) -> Markup { span class="view-meta muted" { (views) " views" } } nav class="top-nav" { - a href="/login" { "login" } + @if let Some(name) = nav_user { + span class="top-nav-user" data-testid="nav-user" { (name) } + a href="/login" { "account" } + form class="top-nav-logout" method="post" action="/auth/logout" data-navigate="full" { + button type="submit" data-testid="nav-logout" { "log out" } + } + } @else { + a href="/login" data-testid="nav-login" { "login" } + } } div id="errors" {} (body) @@ -481,10 +491,11 @@ pub fn input_panel(query: &str, error: Option<&str>) -> Markup { } } -async fn item_page(state: AppState, uri: Uri, item: ItemId) -> Markup { +async fn item_page(state: AppState, uri: Uri, item: ItemId, jar: CookieJar) -> Markup { let path = uri.path().to_string(); state.views.increment(path.clone()); let views = state.views.get_views(&path); + let nav_user = nav_pseudonym(state.projection_store.db(), &jar); let tree = state .scope_tree(&item) @@ -513,16 +524,24 @@ async fn item_page(state: AppState, uri: Uri, item: ItemId) -> Markup { (ranking_panel(&item, node, &tree)) } }; - layout("sorter2", body, views) + layout("sorter2", body, views, nav_user.as_deref()) } -pub async fn home(State(state): State, uri: Uri) -> impl IntoResponse { - item_page(state, uri, ItemId::root()).await +pub async fn home( + State(state): State, + jar: CookieJar, + uri: Uri, +) -> impl IntoResponse { + item_page(state, uri, ItemId::root(), jar).await } -pub async fn browse(State(state): State, uri: Uri) -> impl IntoResponse { +pub async fn browse( + State(state): State, + jar: CookieJar, + uri: Uri, +) -> impl IntoResponse { let item = ItemId::from_browse_uri(uri.path()).unwrap_or(ItemId::root()); - item_page(state, uri, item).await + item_page(state, uri, item, jar).await } #[cfg(test)] diff --git a/server/src/html/vote.rs b/server/src/html/vote.rs index 3aa00c417c89a9cab3417c650b50ed7c73f08e20..cadbec188b17a48a63b269c0e2fa2ea8ffedd7ed 100644 --- a/server/src/html/vote.rs +++ b/server/src/html/vote.rs @@ -4,11 +4,13 @@ use axum::{ extract::{Query, State}, response::{Html, IntoResponse}, }; +use axum_extra::extract::cookie::CookieJar; use maud::{html, Markup}; use serde::Deserialize; use std::collections::HashSet; use crate::{ + auth::nav_pseudonym, fetch::html::entity_section, form_template::template_json_compact, html::{ranking_panel_with_highlights, scope_theme_style, JsBuilder}, @@ -262,6 +264,7 @@ fn suggest_next( pub async fn vote_page( State(state): State, + jar: CookieJar, Query(q): Query, ) -> impl IntoResponse { let parent = parse_item_param(&q.parent); @@ -329,8 +332,9 @@ pub async fn vote_page( let path = format!("/vote?parent={}", urlencoding::encode(parent.as_str())); state.views.increment(path.clone()); let views = state.views.get_views(&path); + let nav_user = nav_pseudonym(state.projection_store.db(), &jar); - Html(layout(&title, body, views).into_string()).into_response() + Html(layout(&title, body, views, nav_user.as_deref()).into_string()).into_response() } #[cfg(test)] diff --git a/server/static/sorter.css b/server/static/sorter.css index e66a1e6c1acc473c8ff1ddb1e16e75a82d33741c..257280b6b490c65222e580a325b77351cac6cc6b 100644 --- a/server/static/sorter.css +++ b/server/static/sorter.css @@ -34,6 +34,47 @@ body { font-size: 0.75rem; } +.top-nav { + display: flex; + align-items: center; + justify-content: flex-end; + gap: 0.75rem; + padding: 0.5rem 1rem; + font-size: 0.875rem; +} + +.top-nav a { + color: var(--muted); + text-decoration: none; +} + +.top-nav a:hover { + color: var(--fg); +} + +.top-nav-user { + color: var(--fg); + font-weight: 600; +} + +.top-nav-logout { + display: inline; + margin: 0; +} + +.top-nav-logout button { + background: none; + border: none; + padding: 0; + color: var(--muted); + font: inherit; + cursor: pointer; +} + +.top-nav-logout button:hover { + color: var(--fg); +} + .btn-primary { background: var(--accent); color: var(--accent-fg, #0f1115); diff --git a/test/support/harness.clj b/test/support/harness.clj index 3f05951f418258642dcacb4a10ccccc8bfbe8748..4505ece5aa193e826ca61c52f1467d252080d66b 100644 --- a/test/support/harness.clj +++ b/test/support/harness.clj @@ -43,6 +43,7 @@ "SORTER2_VIEWS_LOG" (str data-dir "/views.jsonl") "PORT" (str app-port) "SORTER2_BASE_URL" (str "http://127.0.0.1:" app-port) + "SORTER2_ALLOW_MOCK_OAUTH" "1" "GITHUB_CLIENT_ID" "test-client" "GITHUB_CLIENT_SECRET" "test-secret" "GITHUB_OAUTH_BASE" (str "http://127.0.0.1:" oauth-port) Side B — contributor: tommy-mor Side B — commit message: [c94456ff] Make feed catch-up stable and permission-aware Anchor implicit feeds to durable ingest order and cover multi-user private-room visibility so concurrent posts are not missed or leaked. Co-authored-by: Cursor Side B — unified diff (full patch): diff --git a/cli/src/main.rs b/cli/src/main.rs index abb5a55b49f60fe28fbfd4ec02715cb94ea0b4ec..c4f1494df3aedbd8b883aea6249579aa8336abfe 100644 --- a/cli/src/main.rs +++ b/cli/src/main.rs @@ -878,6 +878,30 @@ mod tests { "graph: 4 items, 3/6 pairs (50.0% density), 1 component, connected" ); } + + #[test] + fn feed_without_since_uses_logged_in_delegate_from_env() { + let key = "SLUG_DELEGATE"; + let previous = std::env::var_os(key); + let expected = "00000000-0000-0000-0000-0000000000ee:test:local/model"; + std::env::set_var(key, expected); + + let cli = Cli::try_parse_from(["slugsocial", "feed"]).expect("parse feed"); + + match previous { + Some(value) => std::env::set_var(key, value), + None => std::env::remove_var(key), + } + match cli.cmd { + Some(Command::Feed { + delegate, since, .. + }) => { + assert_eq!(delegate.as_deref(), Some(expected)); + assert!(since.is_none()); + } + _ => panic!("expected feed command"), + } + } } async fn run_scoped(base: &str, room: &str, sub: ScopedCmd) -> Result<()> { @@ -1626,7 +1650,15 @@ async fn run() -> Result<()> { } else { for p in &resp.posts { let ago = slug_types::timeago::timeago(now_ms, p.ts); - println!("", p.id, ago); + let thread_attr = p + .thread + .as_deref() + .map(|thread| format!(" thread=\"{thread}\"")) + .unwrap_or_default(); + println!( + "", + p.id, ago, p.room, thread_attr + ); print!("{}", p.body); if !p.body.ends_with('\n') { println!(); } println!(""); diff --git a/server/src/api/rpc.rs b/server/src/api/rpc.rs index afc4f95bef160c1e38ecff2c096d6440cd94e2b3..46d748f918d9b225acc4ecedfe5a1793089407b5 100644 --- a/server/src/api/rpc.rs +++ b/server/src/api/rpc.rs @@ -72,6 +72,80 @@ fn can_view_scope(reduced: &ReducerState, scope: &ScopeId, principal: Option<&st } } +/// Build a feed in durable ingest order. +/// +/// An implicit feed boundary is an ingest position, not only its millisecond timestamp. Two users +/// can post in the same millisecond, and wall-clock timestamps can move backwards during replay. +/// Explicit `since` remains a timestamp query for API compatibility, but scans the whole ordered +/// ledger rather than assuming timestamps are monotonic. +fn rpc_feed( + reduced: &ReducerState, + viewer: &str, + delegate: Option, + requested_since: Option, + implicit_anchor: Option<(usize, i64)>, + limit: usize, +) -> FeedResponse { + let since = requested_since.or_else(|| implicit_anchor.map(|(_, ts)| ts)); + let implicit_anchor_index = requested_since + .is_none() + .then(|| implicit_anchor.map(|(index, _)| index)) + .flatten(); + + let matching: Vec<&str> = reduced + .ingests_ordered + .iter() + .enumerate() + .rev() + .filter(|(index, id)| { + reduced.ingests_by_id.get(id.as_str()).is_some_and(|ing| { + match requested_since { + Some(cutoff) => ing.ts > cutoff, + None => implicit_anchor_index.is_none_or(|anchor| *index > anchor), + } + }) + }) + .map(|(_, id)| id.as_str()) + .filter(|id| { + reduced.ingests_by_id.get(*id).is_some_and(|ing| { + let scope = scope_from_room_wire(&ing.room_id); + can_view_scope(reduced, &scope, Some(viewer)) + }) + }) + .filter(|id| !reduced.redacted_posts.contains(*id)) + .collect(); + + let total = matching.len(); + let posts = matching + .into_iter() + .take(limit) + .filter_map(|id| reduced.ingests_by_id.get(id)) + .map(|ing| { + let scope = scope_from_room_wire(&ing.room_id); + let thread_post_index = reduced.try_thread_post_index_chronological( + &scope, + &ing.thread_tag, + &ing.id, + ); + FeedPost { + ts: ing.ts, + id: ing.id.clone(), + room: ing.room_id.clone(), + thread: Some(ing.thread_tag.clone()), + thread_post_index, + body: ing.raw.clone(), + } + }) + .collect(); + + FeedResponse { + delegate, + since, + posts, + total, + } +} + fn principal_from_optional_bearer(headers: &HeaderMap, reduced: &ReducerState) -> Result, RpcErr> { if headers.contains_key(axum::http::header::AUTHORIZATION) { verify_bearer_principal(headers, reduced) @@ -1506,60 +1580,27 @@ pub async fn handle_rpc_batch( Some("this delegate is not bound to your signed-in account".into()), ) } else { - let since_default = reduced + let implicit_anchor = reduced .ingests_ordered .iter() + .enumerate() .rev() - .filter_map(|id| reduced.ingests_by_id.get(id)) - .find(|ing| { - if ing.delegate.as_deref() != Some(delegate_stored.as_str()) { - return false; - } - let scope = scope_from_room_wire(&ing.room_id); - can_view_scope(&reduced, &scope, Some(viewer.as_str())) - }) - .map(|ing| ing.ts); - let since = since.or(since_default); - let cutoff = since.unwrap_or(0); - let limit = limit.unwrap_or(DEFAULT_LIMIT).min(MAX_LIMIT); - let matching: Vec<&str> = reduced.ingests_ordered.iter().rev() - .map(|id| id.as_str()) - .take_while(|id| reduced.ingests_by_id.get(*id).is_some_and(|ing| ing.ts > cutoff)) - .filter(|id| { - reduced.ingests_by_id.get(*id).is_some_and(|ing| { - let scope = scope_from_room_wire(&ing.room_id); - can_view_scope(&reduced, &scope, Some(viewer.as_str())) + .find_map(|(index, id)| { + reduced.ingests_by_id.get(id).and_then(|ing| { + (ing.delegate.as_deref() + == Some(delegate_stored.as_str())) + .then_some((index, ing.ts)) }) - }) - .filter(|id| !reduced.redacted_posts.contains(*id)) - .collect(); - let total = matching.len(); - let posts: Vec = matching.into_iter() - .take(limit) - .filter_map(|id| reduced.ingests_by_id.get(id)) - .map(|ing| { - let scope = scope_from_room_wire(&ing.room_id); - let thread_post_index = reduced - .try_thread_post_index_chronological( - &scope, - &ing.thread_tag, - &ing.id, - ); - FeedPost { - ts: ing.ts, - id: ing.id.clone(), - thread: Some(ing.thread_tag.clone()), - thread_post_index, - body: ing.raw.clone(), - } - }) - .collect(); - line_ok(RpcResult::Feed(FeedResponse { - delegate: Some(delegate_stored), + }); + let limit = limit.unwrap_or(DEFAULT_LIMIT).min(MAX_LIMIT); + line_ok(RpcResult::Feed(rpc_feed( + &reduced, + &viewer, + Some(delegate_stored), since, - posts, - total, - })) + implicit_anchor, + limit, + ))) }; drop(reduced); line @@ -1567,60 +1608,25 @@ pub async fn handle_rpc_batch( None => { // Session catch-up: last time *you* posted anything (delegate or not), so revisiting // an old chat with only a token still gets a sane cutoff. - let since_default = reduced + let implicit_anchor = reduced .ingests_ordered .iter() + .enumerate() .rev() - .filter_map(|id| reduced.ingests_by_id.get(id)) - .find(|ing| { - if ing.principal != viewer { - return false; - } - let scope = scope_from_room_wire(&ing.room_id); - can_view_scope(&reduced, &scope, Some(viewer.as_str())) - }) - .map(|ing| ing.ts); - let since = since.or(since_default); - let cutoff = since.unwrap_or(0); - let limit = limit.unwrap_or(DEFAULT_LIMIT).min(MAX_LIMIT); - let matching: Vec<&str> = reduced.ingests_ordered.iter().rev() - .map(|id| id.as_str()) - .take_while(|id| reduced.ingests_by_id.get(*id).is_some_and(|ing| ing.ts > cutoff)) - .filter(|id| { - reduced.ingests_by_id.get(*id).is_some_and(|ing| { - let scope = scope_from_room_wire(&ing.room_id); - can_view_scope(&reduced, &scope, Some(viewer.as_str())) + .find_map(|(index, id)| { + reduced.ingests_by_id.get(id).and_then(|ing| { + (ing.principal == viewer).then_some((index, ing.ts)) }) - }) - .filter(|id| !reduced.redacted_posts.contains(*id)) - .collect(); - let total = matching.len(); - let posts: Vec = matching.into_iter() - .take(limit) - .filter_map(|id| reduced.ingests_by_id.get(id)) - .map(|ing| { - let scope = scope_from_room_wire(&ing.room_id); - let thread_post_index = reduced - .try_thread_post_index_chronological( - &scope, - &ing.thread_tag, - &ing.id, - ); - FeedPost { - ts: ing.ts, - id: ing.id.clone(), - thread: Some(ing.thread_tag.clone()), - thread_post_index, - body: ing.raw.clone(), - } - }) - .collect(); - let line = line_ok(RpcResult::Feed(FeedResponse { - delegate: None, + }); + let limit = limit.unwrap_or(DEFAULT_LIMIT).min(MAX_LIMIT); + let line = line_ok(RpcResult::Feed(rpc_feed( + &reduced, + &viewer, + None, since, - posts, - total, - })); + implicit_anchor, + limit, + ))); drop(reduced); line } diff --git a/server/tests/integration_rooms.rs b/server/tests/integration_rooms.rs index a3b378267e090a016a7e663f103a0dd9f482690b..e623523ba352ce456674a399542c689505760b78 100644 --- a/server/tests/integration_rooms.rs +++ b/server/tests/integration_rooms.rs @@ -1,6 +1,9 @@ mod support; use slug_types::room_route_segment; +use slugsocial_server::events::{ + AgentBound, Event, GrantAdded, GrantRevoked, Ingest, RoomCreated, ThreadCapability, +}; use support::*; #[tokio::test] @@ -473,6 +476,247 @@ async fn test_feed_without_delegate_uses_principal_last_post_including_delegate( ); } +#[tokio::test] +async fn test_feed_uses_delegate_ingest_position_when_multi_user_timestamps_collide() { + let (addr, _tmp, _log, state, _handle) = create_test_server_with_state().await; + seed_test_identity(&state, "bob", "bobtok", "bobsecret").await; + let client = reqwest::Client::new(); + let alice_delegate = + "00000000-0000-0000-0000-0000000000c1:feedtest:local/alice-model"; + let bob_delegate = + "00000000-0000-0000-0000-0000000000c2:feedtest:local/bob-model"; + + { + let mut reduced = state.reduced.write().await; + for (agent, username) in [ + (alice_delegate, "testuser"), + (bob_delegate, "bob"), + ] { + reduced.apply_event(Event::AgentBound(AgentBound { + ts: 1, + agent: agent.to_string(), + username: username.to_string(), + })); + } + reduced.apply_event(Event::Ingest(Ingest { + ts: 100, + id: "alice-anchor".into(), + raw: "alice anchor".into(), + principal: "testuser".into(), + delegate: Some(alice_delegate.into()), + room_id: "public".into(), + thread_tag: "feed-order".into(), + })); + reduced.apply_event(Event::Ingest(Ingest { + ts: 100, + id: "bob-same-millisecond".into(), + raw: "bob same-millisecond change".into(), + principal: "bob".into(), + delegate: Some(bob_delegate.into()), + room_id: "public".into(), + thread_tag: "feed-order".into(), + })); + // Event-log order remains authoritative even if the wall clock moves backwards. + reduced.apply_event(Event::Ingest(Ingest { + ts: 99, + id: "bob-clock-rollback".into(), + raw: "bob change after clock rollback".into(), + principal: "bob".into(), + delegate: Some(bob_delegate.into()), + room_id: "public".into(), + thread_tag: "feed-order".into(), + })); + } + + let feed = rpc_batch( + &client, + addr, + Some(&test_bearer()), + serde_json::json!([{ + "GetFeed": { "delegate": alice_delegate, "limit": 20 } + }]), + ) + .await; + let posts = feed["results"][0]["result"]["Feed"]["posts"] + .as_array() + .unwrap(); + let ids: Vec<&str> = posts.iter().filter_map(|p| p["id"].as_str()).collect(); + assert_eq!( + ids, + ["bob-clock-rollback", "bob-same-millisecond"], + "implicit feed cutoff must use append position, not timestamp" + ); +} + +#[tokio::test] +async fn test_feed_multi_user_private_room_visibility_and_revoked_anchor_access() { + let (addr, _tmp, _log, state, _handle) = create_test_server_with_state().await; + seed_test_identity(&state, "bob", "bobtok", "bobsecret").await; + seed_test_identity(&state, "carol", "caroltok", "carolsecret").await; + let client = reqwest::Client::new(); + let alice_delegate = + "00000000-0000-0000-0000-0000000000d1:feedtest:local/alice-model"; + let bob_delegate = + "00000000-0000-0000-0000-0000000000d2:feedtest:local/bob-model"; + let room_alice = "alice01/alice-room"; + let room_bob = "bob0001/bob-room"; + + { + let mut reduced = state.reduced.write().await; + for (agent, username) in [ + (alice_delegate, "testuser"), + (bob_delegate, "bob"), + ] { + reduced.apply_event(Event::AgentBound(AgentBound { + ts: 1, + agent: agent.to_string(), + username: username.to_string(), + })); + } + for (room_id, slug, owner) in [ + (room_alice, "alice-room", "testuser"), + (room_bob, "bob-room", "bob"), + ] { + reduced.apply_event(Event::RoomCreated(RoomCreated { + ts: 2, + room_id: room_id.to_string(), + slug: slug.to_string(), + owner: owner.to_string(), + })); + reduced.apply_event(Event::GrantAdded(GrantAdded { + ts: 2, + room_id: room_id.to_string(), + username: owner.to_string(), + capabilities: vec![ThreadCapability::View], + granted_by: owner.to_string(), + })); + } + + let mut add_ingest = + |ts, id: &str, raw: &str, principal: &str, delegate: Option<&str>, room: &str| { + reduced.apply_event(Event::Ingest(Ingest { + ts, + id: id.into(), + raw: raw.into(), + principal: principal.into(), + delegate: delegate.map(str::to_string), + room_id: room.into(), + thread_tag: "feed-permissions".into(), + })); + }; + add_ingest(10, "prehistory", "must remain before alice anchor", "carol", None, "public"); + add_ingest( + 11, + "alice-private-anchor", + "alice last posted here", + "testuser", + Some(alice_delegate), + room_alice, + ); + add_ingest( + 12, + "bob-public-anchor", + "bob last posted here", + "bob", + Some(bob_delegate), + "public", + ); + add_ingest(13, "alice-room-change", "visible only to alice", "carol", None, room_alice); + add_ingest(14, "bob-room-change", "visible only to bob", "carol", None, room_bob); + add_ingest(15, "public-change", "visible to everyone", "carol", None, "public"); + + reduced.apply_event(Event::GrantRevoked(GrantRevoked { + ts: 16, + room_id: room_alice.into(), + username: "testuser".into(), + capabilities: vec![ThreadCapability::View], + revoked_by: "testuser".into(), + })); + } + + let alice_feed = rpc_batch( + &client, + addr, + Some(&test_bearer()), + serde_json::json!([{ + "GetFeed": { "delegate": alice_delegate, "limit": 20 } + }]), + ) + .await; + let alice_posts = alice_feed["results"][0]["result"]["Feed"]["posts"] + .as_array() + .unwrap(); + let alice_ids: Vec<&str> = alice_posts + .iter() + .filter_map(|p| p["id"].as_str()) + .collect(); + assert_eq!( + alice_ids, + ["public-change", "bob-public-anchor"], + "revoked private content must be hidden without moving the delegate anchor backwards" + ); + assert!(!alice_ids.contains(&"prehistory")); + assert!( + alice_posts + .iter() + .all(|post| post["room"].as_str() == Some("public")), + "private posts must not leak and every feed post must identify its room" + ); + + let bob_bearer = test_bearer_for("bobtok", "bobsecret"); + let bob_feed = rpc_batch( + &client, + addr, + Some(&bob_bearer), + serde_json::json!([{ + "GetFeed": { "delegate": bob_delegate, "limit": 20 } + }]), + ) + .await; + let bob_ids: Vec<&str> = bob_feed["results"][0]["result"]["Feed"]["posts"] + .as_array() + .unwrap() + .iter() + .filter_map(|p| p["id"].as_str()) + .collect(); + assert_eq!(bob_ids, ["public-change", "bob-room-change"]); + assert_eq!( + bob_feed["results"][0]["result"]["Feed"]["posts"][1]["room"], + room_bob + ); + + // Restoring View exposes only changes after the same stable delegate anchor. + { + let mut reduced = state.reduced.write().await; + reduced.apply_event(Event::GrantAdded(GrantAdded { + ts: 17, + room_id: room_alice.into(), + username: "testuser".into(), + capabilities: vec![ThreadCapability::View], + granted_by: "testuser".into(), + })); + } + let restored = rpc_batch( + &client, + addr, + Some(&test_bearer()), + serde_json::json!([{ + "GetFeed": { "delegate": alice_delegate, "limit": 20 } + }]), + ) + .await; + let restored_ids: Vec<&str> = restored["results"][0]["result"]["Feed"]["posts"] + .as_array() + .unwrap() + .iter() + .filter_map(|p| p["id"].as_str()) + .collect(); + assert_eq!( + restored_ids, + ["public-change", "alice-room-change", "bob-public-anchor"] + ); +} + #[tokio::test] async fn test_private_room_thread_urls_use_t_segment() { let (addr, _tmp, _log, _handle) = create_test_server().await; diff --git a/server/tests/support/mod.rs b/server/tests/support/mod.rs index 4a620eaa875e7a1145f2a4e82cc4ff2be21d5345..a4320e991763617c1a760efad4b621977e2b74d0 100644 --- a/server/tests/support/mod.rs +++ b/server/tests/support/mod.rs @@ -20,8 +20,10 @@ pub fn sha256_hex(s: &str) -> String { /// Fixed bearer for integration tests (`TokenIssued` seeded into reducer in `create_test_server`). pub fn test_bearer() -> String { - let token_id = "testtok"; - let secret = "secret"; + test_bearer_for("testtok", "secret") +} + +pub fn test_bearer_for(token_id: &str, secret: &str) -> String { format!("slug_{token_id}_{secret}") } @@ -71,19 +73,27 @@ pub async fn rpc_batch( } pub async fn seed_test_token(state: &AppState) { + seed_test_identity(state, "testuser", "testtok", "secret").await; +} + +/// Add a distinct principal and bearer to a running integration-test server. +pub async fn seed_test_identity( + state: &AppState, + username: &str, + token_id: &str, + secret: &str, +) { let registered = Event::UserRegistered(UserRegistered { ts: 0, - username: "testuser".to_string(), + username: username.to_string(), provider: "test".to_string(), - provider_id: "testuser".to_string(), + provider_id: username.to_string(), }); - let token_id = "testtok"; - let secret = "secret"; let salt = "salt"; let token_hash = sha256_hex(&format!("{salt}:{secret}")); let ev = Event::TokenIssued(TokenIssued { ts: 0, - username: "testuser".to_string(), + username: username.to_string(), token_id: token_id.to_string(), token_hash, salt: salt.to_string(), diff --git a/types/src/lib.rs b/types/src/lib.rs index 211493935d19582607f5c87fb492faf47bdc6f53..cd6f94c60a4e1c3bfbce13bc0404b803f5f57c6d 100644 --- a/types/src/lib.rs +++ b/types/src/lib.rs @@ -264,6 +264,9 @@ pub struct FeedResponse { pub struct FeedPost { pub ts: i64, pub id: String, + /// Permission scope containing the post: `"public"` or a private room id. + #[serde(default)] + pub room: String, /// Primary thread tag (without #), if the ingest declared one. #[serde(skip_serializing_if = "Option::is_none")] pub thread: Option,