Gist access policy + production hardening (gist-001)

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/<id>.git clone endpoint/materialized gist git storage is not implemented; focused authenticated Playwright/a11y for gist flow still missing.

crates/api/src/domain/gists.rs

@@ -181,11 +181,21 @@ pub async fn list_gists(
     }
     let total = count_query.fetch_one(pool).await?.get::<i64, _>("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<GistDetail, GistError> {
     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")

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<PgPool> {
+    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<Value>,
+) -> (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 <script>alert(1)</script> 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("<script>alert"),
+        "embed must escape gist metadata"
+    );
+
+    let (status, _, unauth_create) = json_request(
+        app.clone(),
+        Method::POST,
+        "/api/gists",
+        None,
+        Some(json!({"files":[{"filename":"x.txt","content":"x"}]})),
+    )
+    .await;
+    assert_eq!(status, StatusCode::UNAUTHORIZED);
+    assert_eq!(unauth_create["error"]["code"], "not_authenticated");
+
+    let (status, _, invalid) = json_request(
+        app,
+        Method::POST,
+        "/api/gists",
+        Some(&owner_cookie),
+        Some(json!({"files":[]})),
+    )
+    .await;
+    assert_eq!(status, StatusCode::UNPROCESSABLE_ENTITY);
+    assert_eq!(invalid["error"]["code"], "validation_failed");
+}

prd.json

@@ -2312,7 +2312,8 @@
       "markdown-001",
       "highlight-001"
     ],
-    "build_pass": true
+    "build_pass": true,
+    "qa_pass": false
   },
   {
     "id": "ai-001",