diff --git a/README.md b/README.md index a26271978372a1f819a1899e1d3254d04ebb062a..20083a3371821f9ca2a1e7e608cf40b6d2d41d04 100644 --- a/README.md +++ b/README.md @@ -348,3 +348,17 @@ rotation, revocation, stable event selection, and Atom and RSS parsing. Read the [scoped feeds architectural decision record](docs/adr/0019-scoped-feeds.md) for the token, authorization, and ordering contracts. + +## Milestone 4.5 gate + +Run the metadata search gate: + +```text +./scripts/check-m4-5 +``` + +This command tests bounded repository and issue metadata search, public and +private visibility, current collaborator permission, stable result identity, +query validation, and the representative index workload. Read the +[bounded metadata search architectural decision record](docs/adr/0020-bounded-metadata-search.md) +for the limits, authorization, and index decision. diff --git a/docs/adr/0020-bounded-metadata-search.md b/docs/adr/0020-bounded-metadata-search.md new file mode 100644 index 0000000000000000000000000000000000000000..03093d87506fefef5395ce21c4fa86d95c9fa6fb --- /dev/null +++ b/docs/adr/0020-bounded-metadata-search.md @@ -1,0 +1,64 @@ +# Architectural decision record 0020: Bounded metadata search + +Status: Accepted + +Date: 2026-07-23 + +## Context + +An account must be able to search repository and issue metadata that it can +read. An anonymous user must be able to search public metadata. A permanent +index adds derived state, schema work, and update work. The first search +implementation must show that this additional state is necessary. + +## Decision + +Add a fixed-text, case-insensitive metadata search at `/search`. Search active +repositories, issue titles and bodies, and issue comment bodies. Return one +result for an issue when its issue record or one of its comments matches. Use a +stable repository or issue URL for each result. + +Apply the repository authorization query before the metadata scan. An +anonymous search can read only public repositories. An authenticated search +can also read private repositories that the account owns or has permission to +read. Do not show an archived repository. Check current account and +collaborator state for each search. + +Use these limits for one search: + +- Accept a maximum of 256 query bytes. +- Scan a maximum of 10,000 metadata records. +- Read a maximum of 8 MiB of title and body content. +- Return a maximum of 100 results. +- Stop the application scan after 500 ms. + +Return the completed prefix and tell the user when a limit stops the search. +Run the synchronous SQLite work as a bounded blocking job. Use a GET form that +operates without JavaScript. Read one candidate at a time. Limit each title and +body field to 8 MiB plus one byte in the SQL result. The additional byte shows +that the content limit was reached. Thus, candidate text uses a maximum of 16 +MiB plus two bytes before the application stops the scan. + +Do not add an index. The release-mode workload has one public repository and +9,999 issues. It searches 10,000 records and 449,972 bytes for text in the last +issue. On the development host, the search took 14.4 ms on 2026-07-23. The +index threshold is 250 ms for this workload. Keep the workload in +`tests/metadata_search.rs` so that a contributor can measure it again. +The retained candidate text threshold is 32 MiB. The field limits keep the +implementation below this threshold. + +## Evidence + +The storage test proves that the authorization query filters private metadata. +The search test covers public and private metadata, collaborator permission, +case-insensitive matching, comment result deduplication, query validation, and +stable result identity after a restart. The public-route test covers the form, +anonymous search, authenticated private search, hidden private results, and an +invalid query. The release-mode workload records the index decision. + +## Consequences + +Each request reads canonical SQLite metadata, so search results cannot become +out of date. A large instance can return an incomplete result when it reaches a +limit. Add a derivable embedded index only when repeat measurements exceed the +250 ms time threshold or the 32 MiB retained candidate text threshold. diff --git a/scripts/check-m4-5 b/scripts/check-m4-5 new file mode 100755 index 0000000000000000000000000000000000000000..1bde6dc58aad17f36330179d755936da3b9b1f64 --- /dev/null +++ b/scripts/check-m4-5 @@ -1,0 +1,7 @@ +#!/bin/sh +set -eu + +./scripts/check +cargo test --locked --release --test metadata_search +cargo test --locked --release --test metadata_search measures_bounded_metadata_search_without_an_index -- --ignored --nocapture +cargo test --locked --release --test public_routes runs_the_complete_issue_workflow_without_javascript diff --git a/src/http/metadata_search.rs b/src/http/metadata_search.rs new file mode 100644 index 0000000000000000000000000000000000000000..223673f2bc368659f3ac7d42467db3f6da29b442 --- /dev/null +++ b/src/http/metadata_search.rs @@ -1,0 +1,140 @@ +use askama::Template; +use axum::Router; +use axum::extract::{Extension, Query, State}; +use axum::http::StatusCode; +use axum::response::Response; +use axum::routing::get; +use serde::Deserialize; + +use crate::search::{MetadataSearchError, MetadataSearchResult}; +use crate::store::StoreError; + +use super::{RequestActor, RequestId, WebState, render, render_error}; + +pub(super) fn routes() -> Router { + Router::new().route("/search", get(search)) +} + +async fn search( + State(state): State, + Extension(request_id): Extension, + Extension(actor): Extension, + Query(query): Query, +) -> Response { + let Some(value) = query.q else { + return search_page(&request_id.0, "", None); + }; + let Some(service) = state.search.clone() else { + return search_internal(&request_id.0); + }; + let value_for_search = value.clone(); + let result = search_job(state, move || { + service.search(actor.0.as_deref(), &value_for_search) + }) + .await; + match result { + Ok(outcome) => search_page(&request_id.0, &value, Some(outcome)), + Err(MetadataSearchError::InvalidQuery | MetadataSearchError::Auth(_)) => render_error( + StatusCode::BAD_REQUEST, + &request_id.0, + "Search error", + "The metadata search query is not valid.", + ), + Err(_) => search_internal(&request_id.0), + } +} + +async fn search_job( + state: WebState, + operation: impl FnOnce() -> Result + Send + 'static, +) -> Result { + let permit = state.jobs.acquire_owned().await.map_err(|_| { + MetadataSearchError::Store(StoreError::Integrity( + "search worker pool is unavailable".to_owned(), + )) + })?; + tokio::task::spawn_blocking(move || { + let _permit = permit; + operation() + }) + .await + .map_err(|_| { + MetadataSearchError::Store(StoreError::Integrity("search worker failed".to_owned())) + })? +} + +fn search_page( + request_id: &str, + query: &str, + outcome: Option, +) -> Response { + let searched = outcome.is_some(); + let (rows_scanned, bytes_scanned, truncated, results) = outcome.map_or_else( + || (0, 0, false, Vec::new()), + |outcome| { + debug_assert_eq!(outcome.query, query.trim()); + ( + outcome.rows_scanned, + outcome.bytes_scanned, + outcome.truncated, + outcome.results, + ) + }, + ); + render( + StatusCode::OK, + &MetadataSearchTemplate { + request_id, + query, + searched, + rows_scanned, + bytes_scanned, + truncated, + results: results.iter().map(result_view).collect(), + }, + ) +} + +fn result_view(result: &MetadataSearchResult) -> MetadataSearchResultView<'_> { + MetadataSearchResultView { + kind: result.kind, + url: &result.url, + title: &result.title, + summary: &result.summary, + stable_id: &result.stable_id, + } +} + +fn search_internal(request_id: &str) -> Response { + render_error( + StatusCode::INTERNAL_SERVER_ERROR, + request_id, + "Search error", + "The metadata search could not be completed.", + ) +} + +#[derive(Deserialize)] +struct SearchQuery { + q: Option, +} + +struct MetadataSearchResultView<'a> { + kind: &'static str, + url: &'a str, + title: &'a str, + summary: &'a str, + stable_id: &'a str, +} + +#[derive(Template)] +#[template(path = "metadata-search.html")] +struct MetadataSearchTemplate<'a> { + request_id: &'a str, + query: &'a str, + searched: bool, + rows_scanned: usize, + bytes_scanned: usize, + truncated: bool, + results: Vec>, +} diff --git a/src/http/mod.rs b/src/http/mod.rs index e4e965678e3aeb2e239503f9de0171d186e65594..75668e8aa82918de2e4170200283775587d460d3 100644 --- a/src/http/mod.rs +++ b/src/http/mod.rs @@ -1,5 +1,6 @@ mod feeds; mod issues; +mod metadata_search; mod public; mod watches; @@ -26,6 +27,7 @@ use crate::feed_token::FeedTokenService; use crate::issue::IssueService; use crate::repository::{RepositoryService, RepositoryServiceError}; +use crate::search::MetadataSearchService; use crate::session::{SessionError, WebLoginService}; use crate::store::StoreError; use crate::watch::WatchService; @@ -50,6 +52,7 @@ repositories: Option, issues: Option, feeds: Option, + search: Option, watches: Option, secure_cookies: bool, } @@ -85,6 +88,7 @@ repositories: None, issues: None, feeds: None, + search: None, watches: None, secure_cookies: false, }, @@ -123,6 +127,7 @@ let repositories = RepositoryService::new(public.database(), public.repository_root()); let issues = IssueService::new(public.database()); let feeds = FeedTokenService::new(public.database()); + let search = MetadataSearchService::new(public.database()); let watches = WatchService::new(public.database()); Self::start_with_state( address, @@ -135,6 +140,7 @@ repositories: Some(repositories), issues: Some(issues), feeds: Some(feeds), + search: Some(search), watches: Some(watches), secure_cookies, }, @@ -181,13 +187,15 @@ repositories: None, issues: None, feeds: None, + search: None, watches: None, secure_cookies: false, }) } fn router_with_state(state: WebState) -> Router { - let repository_routes = watches::routes() + let repository_routes = metadata_search::routes() + .merge(watches::routes()) .merge(issues::routes()) .merge(public::routes()) .layer(middleware::from_fn_with_state( diff --git a/src/main.rs b/src/main.rs index ba1d87e7422f277bf3573637046633b20cae7b96..a17845a01076c2462802b771be506ea1f990442a 100644 --- a/src/main.rs +++ b/src/main.rs @@ -21,6 +21,7 @@ mod markdown; mod policy; mod repository; +mod search; mod serve; mod session; #[allow(dead_code, reason = "the server uses only part of the shared SSH API")] diff --git a/src/search.rs b/src/search.rs new file mode 100644 index 0000000000000000000000000000000000000000..bafe8d66532e08dd800524f399bd89a6bcfad976 --- /dev/null +++ b/src/search.rs @@ -1,0 +1,180 @@ +use std::collections::BTreeSet; +use std::path::{Path, PathBuf}; +use std::time::{Duration, Instant}; + +use thiserror::Error; + +use crate::auth::{AuthError, validate_username}; +use crate::store::{MetadataSearchCandidate, Store, StoreError}; + +pub(crate) const MAX_QUERY_BYTES: usize = 256; +pub(crate) const MAX_SCAN_ROWS: usize = 10_000; +pub(crate) const MAX_SCAN_BYTES: usize = 8 * 1024 * 1024; +pub(crate) const MAX_RESULTS: usize = 100; +pub(crate) const MAX_DURATION: Duration = Duration::from_millis(500); + +#[derive(Clone)] +pub(crate) struct MetadataSearchService { + database: PathBuf, +} + +impl MetadataSearchService { + pub(crate) fn new(database: &Path) -> Self { + Self { + database: database.to_owned(), + } + } + + pub(crate) fn search( + &self, + actor: Option<&str>, + query: &str, + ) -> Result { + if let Some(actor) = actor { + validate_username(actor)?; + } + let query = query.trim(); + if query.is_empty() || query.len() > MAX_QUERY_BYTES || query.chars().any(char::is_control) + { + return Err(MetadataSearchError::InvalidQuery); + } + let started = Instant::now(); + let store = Store::open(&self.database)?; + let needle = query.to_lowercase(); + let mut bytes_scanned = 0_usize; + let mut rows_scanned = 0_usize; + let mut seen = BTreeSet::new(); + let mut results = Vec::new(); + let mut candidate_error = None; + let truncated = store.visit_metadata_search_candidates( + actor, + MAX_SCAN_ROWS, + MAX_SCAN_BYTES, + |candidate| { + if started.elapsed() >= MAX_DURATION { + return false; + } + let Some(bytes) = candidate.title.len().checked_add(candidate.body.len()) else { + candidate_error = Some(MetadataSearchError::Limit); + return false; + }; + if bytes_scanned.saturating_add(bytes) > MAX_SCAN_BYTES { + return false; + } + bytes_scanned += bytes; + rows_scanned += 1; + let (Ok(title), Ok(body)) = ( + std::str::from_utf8(&candidate.title), + std::str::from_utf8(&candidate.body), + ) else { + candidate_error = Some(MetadataSearchError::StoredCandidate); + return false; + }; + if !title.to_lowercase().contains(&needle) && !body.to_lowercase().contains(&needle) + { + return true; + } + let Ok(result) = result_from_candidate(candidate) else { + candidate_error = Some(MetadataSearchError::StoredCandidate); + return false; + }; + if !seen.insert(result.url.clone()) { + return true; + } + if results.len() == MAX_RESULTS { + return false; + } + results.push(result); + true + }, + )?; + if let Some(error) = candidate_error { + return Err(error); + } + + Ok(MetadataSearchOutcome { + query: query.to_owned(), + rows_scanned, + bytes_scanned, + truncated, + results, + }) + } +} + +fn result_from_candidate( + candidate: MetadataSearchCandidate, +) -> Result { + let title = + String::from_utf8(candidate.title).map_err(|_| MetadataSearchError::StoredCandidate)?; + let body = + String::from_utf8(candidate.body).map_err(|_| MetadataSearchError::StoredCandidate)?; + let (kind, url, title) = match candidate.kind.as_str() { + "repository" => ( + "Repository", + format!("/{}/{}", candidate.owner, candidate.repository), + title, + ), + "issue" | "issue-comment" => { + let number = candidate + .issue_number + .ok_or(MetadataSearchError::StoredCandidate)?; + ( + "Issue", + format!( + "/{}/{}/issues/{number}", + candidate.owner, candidate.repository + ), + format!( + "{}/{} #{number}: {}", + candidate.owner, candidate.repository, title + ), + ) + } + _ => return Err(MetadataSearchError::StoredCandidate), + }; + let summary = body + .split_whitespace() + .collect::>() + .join(" ") + .chars() + .take(160) + .collect(); + Ok(MetadataSearchResult { + kind, + url, + title, + summary, + stable_id: candidate.record_id, + }) +} + +pub(crate) struct MetadataSearchOutcome { + pub(crate) query: String, + pub(crate) rows_scanned: usize, + pub(crate) bytes_scanned: usize, + pub(crate) truncated: bool, + pub(crate) results: Vec, +} + +pub(crate) struct MetadataSearchResult { + pub(crate) kind: &'static str, + pub(crate) url: String, + pub(crate) title: String, + pub(crate) summary: String, + pub(crate) stable_id: String, +} + +#[derive(Debug, Error)] +pub(crate) enum MetadataSearchError { + #[error(transparent)] + Auth(#[from] AuthError), + #[error(transparent)] + Store(#[from] StoreError), + #[error("metadata search query is not valid")] + InvalidQuery, + #[error("metadata search reached an arithmetic limit")] + Limit, + #[error("stored metadata search candidate is not valid")] + StoredCandidate, +} diff --git a/src/store/mod.rs b/src/store/mod.rs index f951548112789d6c920e4cdaa587c02ee476402b..6b9c5adaa79cce34454f1d13036c7611880e1101 100644 --- a/src/store/mod.rs +++ b/src/store/mod.rs @@ -2140,6 +2140,86 @@ }) } + pub(crate) fn visit_metadata_search_candidates( + &self, + actor: Option<&str>, + limit: usize, + max_field_bytes: usize, + mut visit: impl FnMut(MetadataSearchCandidate) -> bool, + ) -> Result { + let query_limit = limit.checked_add(1).ok_or(StoreError::EventLimit)?; + let query_limit = i64::try_from(query_limit).map_err(|_| StoreError::EventLimit)?; + let max_field_bytes = max_field_bytes + .checked_add(1) + .and_then(|value| i64::try_from(value).ok()) + .ok_or(StoreError::EventLimit)?; + let mut statement = self.connection.prepare( + "WITH visible_repository AS ( + SELECT repository.id, owner.username AS owner, repository.slug + FROM repository + JOIN account AS owner ON owner.id = repository.owner_account_id + LEFT JOIN account AS actor + ON actor.username = ?1 AND actor.state = 'active' + WHERE repository.state = 'active' + AND (repository.visibility = 'public' + OR repository.owner_account_id = actor.id + OR EXISTS ( + SELECT 1 FROM repository_collaborator + WHERE repository_collaborator.repository_id = repository.id + AND repository_collaborator.account_id = actor.id + )) + ), candidates AS ( + SELECT 'repository' AS kind, visible_repository.id AS record_id, + visible_repository.owner, visible_repository.slug, + NULL AS issue_number, + CAST(visible_repository.owner || '/' || visible_repository.slug AS BLOB) + AS title, + X'' AS body + FROM visible_repository + UNION ALL + SELECT 'issue', issue.id, visible_repository.owner, + visible_repository.slug, issue.number, + COALESCE(substr(CAST(issue.title AS BLOB), 1, ?3), X''), + COALESCE(substr(CAST(issue.body AS BLOB), 1, ?3), X'') + FROM issue + JOIN visible_repository ON visible_repository.id = issue.repository_id + UNION ALL + SELECT 'issue-comment', issue_comment.id, visible_repository.owner, + visible_repository.slug, issue.number, + COALESCE(substr(CAST(issue.title AS BLOB), 1, ?3), X''), + COALESCE(substr(CAST(issue_comment.body AS BLOB), 1, ?3), X'') + FROM issue_comment + JOIN issue ON issue.id = issue_comment.issue_id + JOIN visible_repository ON visible_repository.id = issue.repository_id + ) + SELECT kind, record_id, owner, slug, issue_number, title, body + FROM candidates + ORDER BY owner, slug, kind, issue_number, record_id + LIMIT ?2", + )?; + let mut rows = statement.query(rusqlite::params![actor, query_limit, max_field_bytes])?; + let mut visited = 0_usize; + while let Some(row) = rows.next()? { + if visited == limit { + return Ok(true); + } + visited += 1; + let candidate = MetadataSearchCandidate { + kind: row.get(0)?, + record_id: row.get(1)?, + owner: row.get(2)?, + repository: row.get(3)?, + issue_number: row.get(4)?, + title: row.get(5)?, + body: row.get(6)?, + }; + if !visit(candidate) { + return Ok(true); + } + } + Ok(false) + } + #[allow( dead_code, reason = "some integration tests compile storage without authorization" @@ -2610,6 +2690,16 @@ pub(crate) username: String, pub(crate) target: Option, pub(crate) events: Vec, +} + +pub(crate) struct MetadataSearchCandidate { + pub(crate) kind: String, + pub(crate) record_id: String, + pub(crate) owner: String, + pub(crate) repository: String, + pub(crate) issue_number: Option, + pub(crate) title: Vec, + pub(crate) body: Vec, } pub(crate) struct GitOperationIntent<'a> { diff --git a/templates/base.html b/templates/base.html index 3242bac8ffe1dd5886c57b2e95047b543438a7d9..250279dbab5823374566312a9f8374a37a4fec66 100644 --- a/templates/base.html +++ b/templates/base.html @@ -12,6 +12,7 @@ tit