diff --git a/build-progress.txt b/build-progress.txt index 538f30f6..b285648c 100644 --- a/build-progress.txt +++ b/build-progress.txt @@ -10786,3 +10786,9 @@ Blockers: full make test Cargo passed but broad web Vitest still has unrelated u Status: QA PASS. Fresh controller recovery accepted for Network/Forks and ready for main merge. Evidence: make doctor healthy; make check passed; DB-backed package_detail_contract 4/4, projects_list_contract 1/1, and repository_network_contract 1/1 passed against localhost:55433; focused Network/Forks/API docs Vitest passed 9/9; focused system-Chrome repository-network Playwright passed setup + flow 2/2 in 21.0s. Fixes accepted: package settings camelCase serde IDs, project copy FOR UPDATE OF projects + workflow_key clone + org membership/base-role guard, corrected projects default-open expectation, repository_network_contract rate-limit identity isolation, and repository-network E2E now uses shared auth plus container-backed psql fallback instead of host psql. + +2026-05-24 - gist-001 QA/fix lane +- QA remains blocked (qa_pass=false). Fixed gist update revision query, list pagination placeholders, and secret gist URL-accessible read semantics. +- Added DB-backed api_gists_contract covering create/list/view/edit/revisions/embed, secret unlisted/direct URL behavior, cross-user edit denial, JSON error shapes, and escaped embeds; focused run passed with TEST_DATABASE_URL loaded. +- make check passed; focused GistEditor Vitest passed. Full make test reached and passed gist contract but stopped on unrelated projects_list_contract URL expectation. +- Blocker for qa_pass: /gist/.git clone endpoint/materialized gist git storage is not implemented; focused authenticated Playwright/a11y for gist flow still missing. diff --git a/crates/api/src/domain/gists.rs b/crates/api/src/domain/gists.rs index ff68078e..88729440 100644 --- a/crates/api/src/domain/gists.rs +++ b/crates/api/src/domain/gists.rs @@ -181,11 +181,21 @@ pub async fn list_gists( } let total = count_query.fetch_one(pool).await?.get::("total"); + let limit_param = if actor_user_id.is_some() { + if query.username.is_some() { + 3 + } else { + 2 + } + } else if query.username.is_some() { + 2 + } else { + 1 + }; + let offset_param = limit_param + 1; let list_sql = format!( - "SELECT g.id FROM gists g JOIN users u ON u.id = g.owner_id {where_sql} ORDER BY g.updated_at DESC LIMIT $limit OFFSET $offset" - ) - .replace("$limit", &(if actor_user_id.is_some() { if query.username.is_some() { 3 } else { 2 } } else if query.username.is_some() { 2 } else { 1 }).to_string()) - .replace("$offset", &(if actor_user_id.is_some() { if query.username.is_some() { 4 } else { 3 } } else if query.username.is_some() { 3 } else { 2 }).to_string()); + "SELECT g.id FROM gists g JOIN users u ON u.id = g.owner_id {where_sql} ORDER BY g.updated_at DESC LIMIT ${limit_param} OFFSET ${offset_param}" + ); let mut list_query = sqlx::query(&list_sql); if let Some(actor) = actor_user_id { list_query = list_query.bind(actor); @@ -248,12 +258,14 @@ pub async fn update_gist( return Err(GistError::Forbidden); } let files = validate_files(input.files)?; - sqlx::query("UPDATE gists SET description = $1, is_public = $2 WHERE id = $3") - .bind(trim_optional(input.description)) - .bind(input.is_public.unwrap_or(true)) - .bind(gist_id) - .execute(pool) - .await?; + sqlx::query( + "UPDATE gists SET description = $1, is_public = COALESCE($2, is_public) WHERE id = $3", + ) + .bind(trim_optional(input.description)) + .bind(input.is_public) + .bind(gist_id) + .execute(pool) + .await?; replace_files(pool, gist_id, &files).await?; let version = sqlx::query("SELECT coalesce(max(version), 0)::bigint + 1 AS version FROM gist_revisions WHERE gist_id = $1") .bind(gist_id) @@ -271,9 +283,7 @@ pub async fn get_gist( app_url: &url::Url, ) -> Result { let summary = gist_summary(pool, gist_id, app_url).await?; - if !summary.is_public && Some(summary.owner.id) != actor_user_id { - return Err(GistError::NotFound); - } + // Secret gists are intentionally URL-accessible and unlisted, not auth-gated. let comments = gist_comments(pool, gist_id, app_url).await?; let is_starred = match actor_user_id { Some(actor) => sqlx::query("SELECT EXISTS (SELECT 1 FROM gist_stars WHERE gist_id = $1 AND user_id = $2) AS starred") diff --git a/crates/api/tests/api_gists_contract.rs b/crates/api/tests/api_gists_contract.rs new file mode 100644 index 00000000..274d9d87 --- /dev/null +++ b/crates/api/tests/api_gists_contract.rs @@ -0,0 +1,294 @@ +use axum::{ + body::{to_bytes, Body}, + http::{header, HeaderMap, Method, Request, StatusCode}, +}; +use chrono::{Duration, Utc}; +use opengithub_api::{ + auth::session, + config::{AppConfig, AuthConfig}, + domain::identity::{upsert_session, upsert_user_by_email, User}, +}; +use serde_json::{json, Value}; +use sqlx::PgPool; +use tower::ServiceExt; +use url::Url; +use uuid::Uuid; + +static MIGRATOR: sqlx::migrate::Migrator = sqlx::migrate!("./migrations"); + +async fn database_pool() -> Option { + let database_url = std::env::var("TEST_DATABASE_URL") + .or_else(|_| std::env::var("DATABASE_URL")) + .ok() + .filter(|value| !value.trim().is_empty())?; + let pool = match opengithub_api::db::test_pool_options() + .connect(&database_url) + .await + { + Ok(pool) => pool, + Err(error) => { + eprintln!("skipping gist contract; database connect failed: {error}"); + return None; + } + }; + if let Err(error) = MIGRATOR.run(&pool).await { + eprintln!("skipping gist contract; migrations failed: {error}"); + return None; + } + Some(pool) +} + +fn app_config() -> AppConfig { + AppConfig { + app_url: Url::parse("http://localhost:3015").expect("app URL"), + api_url: Url::parse("http://localhost:3016").expect("api URL"), + auth: Some(AuthConfig { + google_client_id: "google-client-id.apps.googleusercontent.com".to_owned(), + google_client_secret: "google-client-secret".to_owned(), + session_secret: "test-session-secret-with-enough-entropy".to_owned(), + }), + session_cookie_name: "__Host-session".to_owned(), + session_cookie_secure: false, + } +} + +async fn create_user(pool: &PgPool, label: &str) -> User { + let unique = format!("{}{}", label.replace('-', ""), Uuid::new_v4().simple()); + let user = upsert_user_by_email( + pool, + &format!("{unique}@opengithub.local"), + Some(&unique), + None, + ) + .await + .expect("user should upsert"); + sqlx::query("UPDATE users SET username = $1 WHERE id = $2") + .bind(&unique) + .bind(user.id) + .execute(pool) + .await + .expect("username should update"); + user +} + +async fn cookie_header(pool: &PgPool, config: &AppConfig, user: &User) -> String { + let session_id = Uuid::new_v4().to_string(); + let expires_at = Utc::now() + Duration::hours(1); + upsert_session( + pool, + &session_id, + Some(user.id), + json!({"provider":"google"}), + expires_at, + ) + .await + .expect("session should persist"); + let set_cookie = session::set_cookie_header(config, &session_id, expires_at) + .expect("signed cookie should be created"); + let cookie_value = session::cookie_value_from_set_cookie(&set_cookie).expect("cookie value"); + format!("{}={cookie_value}", config.session_cookie_name) +} + +async fn json_request( + app: axum::Router, + method: Method, + uri: &str, + cookie: Option<&str>, + body: Option, +) -> (StatusCode, HeaderMap, Value) { + let mut builder = Request::builder().method(method).uri(uri); + if let Some(cookie) = cookie { + builder = builder.header(header::COOKIE, cookie); + } + let request_body = if let Some(body) = body { + builder = builder.header(header::CONTENT_TYPE, "application/json"); + Body::from(body.to_string()) + } else { + Body::empty() + }; + let response = app + .oneshot(builder.body(request_body).expect("request")) + .await + .expect("response"); + let status = response.status(); + let headers = response.headers().clone(); + let bytes = to_bytes(response.into_body(), usize::MAX) + .await + .expect("body"); + let value = serde_json::from_slice(&bytes) + .unwrap_or_else(|_| json!({"raw": String::from_utf8_lossy(&bytes)})); + (status, headers, value) +} + +async fn raw_request(app: axum::Router, uri: &str) -> (StatusCode, HeaderMap, String) { + let response = app + .oneshot( + Request::builder() + .method(Method::GET) + .uri(uri) + .body(Body::empty()) + .expect("request"), + ) + .await + .expect("response"); + let status = response.status(); + let headers = response.headers().clone(); + let bytes = to_bytes(response.into_body(), usize::MAX) + .await + .expect("body"); + ( + status, + headers, + String::from_utf8_lossy(&bytes).into_owned(), + ) +} + +fn assert_json(headers: &HeaderMap) { + assert!(headers + .get(header::CONTENT_TYPE) + .and_then(|v| v.to_str().ok()) + .is_some_and(|v| v.starts_with("application/json"))); +} + +#[tokio::test] +async fn gist_contract_covers_visibility_edit_revisions_embed_and_errors() { + let Some(pool) = database_pool().await else { + eprintln!("skipping gist contract; set TEST_DATABASE_URL"); + return; + }; + let config = app_config(); + let owner = create_user(&pool, "gistowner").await; + let outsider = create_user(&pool, "gistoutsider").await; + let owner_cookie = cookie_header(&pool, &config, &owner).await; + let outsider_cookie = cookie_header(&pool, &config, &outsider).await; + let app = opengithub_api::build_app_with_config(Some(pool.clone()), config); + + let payload = json!({ + "description": "Secret gist", + "isPublic": false, + "files": [ + {"filename":"main.ts", "content":"export const answer = 42;"}, + {"filename":"notes.md", "content":"# Notes\n"} + ] + }); + let (status, headers, created) = json_request( + app.clone(), + Method::POST, + "/api/gists", + Some(&owner_cookie), + Some(payload), + ) + .await; + assert_eq!(status, StatusCode::CREATED); + assert_json(&headers); + assert_eq!(created["isPublic"], false); + assert_eq!(created["files"].as_array().unwrap().len(), 2); + assert_eq!(created["files"][0]["language"], "TypeScript"); + let gist_id = created["id"].as_str().expect("gist id"); + + let (status, _, mine) = json_request( + app.clone(), + Method::GET, + "/api/gists?scope=mine", + Some(&owner_cookie), + None, + ) + .await; + assert_eq!(status, StatusCode::OK); + assert!(mine["items"] + .as_array() + .unwrap() + .iter() + .any(|item| item["id"].as_str() == Some(gist_id))); + + let (status, _, public_list) = + json_request(app.clone(), Method::GET, "/api/gists/public", None, None).await; + assert_eq!(status, StatusCode::OK); + assert!( + !public_list["items"] + .as_array() + .unwrap() + .iter() + .any(|item| item["id"].as_str() == Some(gist_id)), + "secret gist must be unlisted" + ); + + let (status, _, anonymous_view) = json_request( + app.clone(), + Method::GET, + &format!("/api/gists/{gist_id}"), + None, + None, + ) + .await; + assert_eq!( + status, + StatusCode::OK, + "secret gist URL should be accessible without auth" + ); + assert_eq!(anonymous_view["viewer"]["authenticated"], false); + assert_eq!(anonymous_view["viewer"]["canEdit"], false); + + let (status, _, forbidden) = json_request( + app.clone(), + Method::PATCH, + &format!("/api/gists/{gist_id}"), + Some(&outsider_cookie), + Some(json!({"description":"pwned", "files":[{"filename":"x.txt","content":"x"}]})), + ) + .await; + assert_eq!(status, StatusCode::FORBIDDEN); + assert_eq!(forbidden["error"]["code"], "forbidden"); + assert!(!forbidden.to_string().contains("stack")); + + let (status, _, updated) = json_request(app.clone(), Method::PATCH, &format!("/api/gists/{gist_id}"), Some(&owner_cookie), Some(json!({"description":"Public now", "isPublic": true, "files":[{"filename":"main.ts","content":"export const answer = 43;"}]}))).await; + assert_eq!(status, StatusCode::OK); + assert_eq!(updated["isPublic"], true); + + let (status, _, revisions) = json_request( + app.clone(), + Method::GET, + &format!("/api/gists/{gist_id}/revisions"), + None, + None, + ) + .await; + assert_eq!(status, StatusCode::OK); + assert_eq!(revisions["revisions"].as_array().unwrap().len(), 2); + + let (status, headers, embed) = + raw_request(app.clone(), &format!("/api/gists/{gist_id}/embed.js")).await; + assert_eq!(status, StatusCode::OK); + assert!(headers + .get(header::CONTENT_TYPE) + .and_then(|v| v.to_str().ok()) + .unwrap() + .starts_with("application/javascript")); + assert!(embed.contains("opengithub-gist")); + assert!( + !embed.contains("