From aa2741561a130d5b5fea9016364a53ba6cb2c704 Mon Sep 17 00:00:00 2001
From: pageman <24869+pageman@users.noreply.github.com>
Date: Tue, 25 Aug 2026 19:47:01 +0000
Subject: [PATCH] fix: harden storage coordination and browser auth

---
 crates/walgit-config/src/lib.rs     |  33 ++-------
 crates/walgit-git/src/lib.rs        |  12 +++-
 crates/walgit-server/build.rs       |  14 ++--
 crates/walgit-server/src/health.rs  |  11 +--
 crates/walgit-server/src/lfs.rs     |  11 ++-
 crates/walgit-server/src/prewarm.rs |  35 ++++++---
 crates/walgit-server/src/web/v1.rs  |  42 +++++++++--
 crates/walgit-server/tests/sim.rs   |  14 +++-
 crates/walgit-store/src/gcs.rs      |  36 +++++++---
 crates/walgit-store/src/s3.rs       | 107 +++++++++++++---------------
 web/API.md                          |   4 +-
 web/sdk/README.md                   |   2 +-
 web/sdk/repos.ts                    |   2 +-
 13 files changed, 194 insertions(+), 129 deletions(-)

diff --git a/crates/walgit-config/src/lib.rs b/crates/walgit-config/src/lib.rs
index 9132dc0..672356b 100644
--- a/crates/walgit-config/src/lib.rs
+++ b/crates/walgit-config/src/lib.rs
@@ -9,7 +9,7 @@ pub use bytesize::ByteSize;
 use serde::{Deserialize, Serialize};
 pub use std::str::FromStr;
 
-#[derive(Debug, Clone, Serialize, Deserialize)]
+#[derive(Debug, Clone, Default, Serialize, Deserialize)]
 #[serde(deny_unknown_fields, default)]
 pub struct Config {
     pub server: ServerConfig,
@@ -469,7 +469,7 @@ impl Default for MaintenanceConfig {
 ///   everywhere, so the edge's read-only fallback (D29) works.
 /// * **maintain**: the maintainer loop's units (checkpoints, bundles, compaction,
 ///   fsck/repair) — only on hosts with the `maintain` role.
-/// Placement is by rule, not by capacity: a repo is either this host's or not.
+///   Placement is by rule, not by capacity: a repo is either this host's or not.
 #[derive(Debug, Clone, Serialize, Deserialize)]
 #[serde(deny_unknown_fields, default)]
 pub struct PlacementConfig {
@@ -857,7 +857,7 @@ impl Config {
     /// never `upstream.token_env` (that name is host-only).
     pub fn public_settings_toml(&self) -> Result<String> {
         let mut doc: toml::Table = toml::Table::try_from(self).context("serializing config")?;
-        doc.retain(|k, _| SETTINGS_SECTIONS.iter().any(|s| *s == k));
+        doc.retain(|k, _| SETTINGS_SECTIONS.contains(&k));
         if let Some(toml::Value::Table(u)) = doc.get_mut("upstream") {
             u.remove("token_env");
         }
@@ -1009,25 +1009,6 @@ pub fn repo_listed(list: &[String], owner: &str, name: &str) -> bool {
     })
 }
 
-impl Default for Config {
-    fn default() -> Self {
-        Config {
-            server: ServerConfig::default(),
-            store: StoreConfig::default(),
-            cache: CacheConfig::default(),
-            wal: WalConfig::default(),
-            compaction: CompactionConfig::default(),
-            maintenance: MaintenanceConfig::default(),
-            bundles: BundlesConfig::default(),
-            placement: PlacementConfig::default(),
-            lfs: LfsConfig::default(),
-            upstream: UpstreamConfig::default(),
-            git: GitConfig::default(),
-            telemetry: TelemetryConfig::default(),
-            events: EventsConfig::default(),
-        }
-    }
-}
 impl Default for ServerConfig {
     fn default() -> Self {
         ServerConfig {
@@ -1371,10 +1352,10 @@ impl Config {
             self.server.listen.set_port(port);
             // Standalone / `dev server`: public_url is the origin the browser hits. Keep its
             // port in lockstep with PORT. A real public_url is left alone.
-            if let Some(u) = self.server.public_url.as_mut() {
-                if origin_is_loopback(u) {
-                    *u = rewrite_origin_port(u, port);
-                }
+            if let Some(u) = self.server.public_url.as_mut()
+                && origin_is_loopback(u)
+            {
+                *u = rewrite_origin_port(u, port);
             }
         }
         Ok(ignored)
diff --git a/crates/walgit-git/src/lib.rs b/crates/walgit-git/src/lib.rs
index d48fa36..016fe7b 100644
--- a/crates/walgit-git/src/lib.rs
+++ b/crates/walgit-git/src/lib.rs
@@ -2873,7 +2873,7 @@ fn capabilities_for(service: Service, format: ObjectFormat) -> String {
     let agent = format!("agent=walgit/{WALGIT_VERSION}");
     match service {
         Service::UploadPack => format!(
-            "multi_ack_detailed side-band-64k thin-pack ofs-delta shallow deepen-since deepen-not \
+            "multi_ack_detailed side-band-64k thin-pack ofs-delta shallow \
              no-progress include-tag allow-tip-sha1-in-want allow-reachable-sha1-in-want filter \
              object-format={of} {agent}"
         ),
@@ -3609,6 +3609,16 @@ mod index_pack_trace_tests {
         );
     }
 
+    #[test]
+    fn upload_pack_does_not_advertise_unimplemented_deepen_modes() {
+        let caps = capabilities_for(Service::UploadPack, ObjectFormat::Sha1);
+        assert!(!caps.split_whitespace().any(|c| c == "deepen-since"));
+        assert!(!caps.split_whitespace().any(|c| c == "deepen-not"));
+        assert!(!caps.split_whitespace().any(|c| c == "packfile-uris"));
+        assert!(caps.split_whitespace().any(|c| c == "shallow"));
+        assert!(caps.split_whitespace().any(|c| c == "filter"));
+    }
+
     #[test]
     fn successful_index_pack_records_non_zero_git_ms() {
         let dir = tempfile::tempdir().unwrap();
diff --git a/crates/walgit-server/build.rs b/crates/walgit-server/build.rs
index f7a7eb4..f34bb34 100644
--- a/crates/walgit-server/build.rs
+++ b/crates/walgit-server/build.rs
@@ -1,8 +1,7 @@
 //! Make `cargo build`/`cargo test` work in a fresh checkout without running the
-//! web build. `rust-embed` requires `../../web/dist` to exist at compile time;
-//! when the SPA has not been built (`just web-build`) we drop in a placeholder
-//! `index.html` so the server compiles and the HTML routes still answer 200.
-//! A real `pnpm run build` overwrites the placeholder.
+//! web build. `rust-embed` requires `../../web/dist` to exist at compile time.
+//! Development builds may use a placeholder when the SPA has not been built;
+//! release builds fail instead so a deployable artifact cannot silently omit UI.
 
 use std::fs;
 use std::path::Path;
@@ -15,12 +14,17 @@ fn main() {
     let manifest = Path::new(env!("CARGO_MANIFEST_DIR"));
     let dist = manifest.join("../../web/dist");
     println!("cargo:rerun-if-changed={}", dist.display());
+    println!("cargo:rerun-if-env-changed=PROFILE");
     let index = dist.join("index.html");
     if !index.exists() {
+        let release = std::env::var("PROFILE").as_deref() == Ok("release");
+        if release {
+            panic!("web/dist/index.html is missing in a release build; run `just web-build` first");
+        }
         fs::create_dir_all(&dist).expect("create web/dist");
         fs::write(&index, PLACEHOLDER).expect("write placeholder web/dist/index.html");
         println!(
-            "cargo:warning=web/dist was missing; wrote a placeholder index.html (run `just web-build` for the real UI)"
+            "cargo:warning=web/dist was missing; wrote a development placeholder index.html (run `just web-build` for the real UI)"
         );
     }
 }
diff --git a/crates/walgit-server/src/health.rs b/crates/walgit-server/src/health.rs
index 96c22c3..d32ddf3 100644
--- a/crates/walgit-server/src/health.rs
+++ b/crates/walgit-server/src/health.rs
@@ -18,28 +18,29 @@ pub async fn healthz() -> Json<serde_json::Value> {
     Json(json!({"status": "ok", "version": BUILD_SHA}))
 }
 
-/// 200 once startup prewarm (`cache.prewarm`) finished or
-/// `cache.prewarm_ready_timeout` elapsed; 503 (with what is pending) before.
+/// 200 once every required startup prewarm (`cache.prewarm`) succeeds;
+/// 503 while warming or when any required prewarm failed.
 pub async fn readyz(State(state): State<Arc<AppState>>) -> Response {
     let r = &state.readiness;
     let pending = r.pending.load(std::sync::atomic::Ordering::Acquire);
+    let failures = r.failed.load(std::sync::atomic::Ordering::Acquire);
     // Draining after SIGTERM: tell the edge/LB to stop routing here at once
     // (in-flight work finishes; new object work is refused with Retry-After).
     if walgit_wal::tasks::shutting_down() {
         return (StatusCode::SERVICE_UNAVAILABLE, [(axum::http::header::RETRY_AFTER, "15")], Json(json!({"status": "draining", "version": BUILD_SHA, "running": state.registry.tasks().running_all().len(), "instance": crate::instance::info(&state.cfg)}))).into_response();
     }
-    if r.ready(state.cfg.cache.prewarm_ready_timeout) {
+    if r.ready() {
         // Placement is a liveness fact: deployment verification should assert
         // that each important repository is served by at least one ready host.
         // Return rules, not repository lists; /readyz remains open for probes.
         let p = &state.cfg.placement;
-        return Json(json!({"status": "ready", "version": BUILD_SHA, "prewarm_pending": pending, "instance": crate::instance::info(&state.cfg),
+        return Json(json!({"status": "ready", "version": BUILD_SHA, "prewarm_pending": pending, "prewarm_failed": failures, "instance": crate::instance::info(&state.cfg),
             "placement": {"serve": p.serve, "serve_exclude": p.serve_exclude, "maintain": p.maintain, "maintain_exclude": p.maintain_exclude}})).into_response();
     }
     (
         StatusCode::SERVICE_UNAVAILABLE,
         // Unauthenticated (startup probe): counts only, no repo names.
-        Json(json!({"status": "warming", "version": BUILD_SHA, "prewarm_pending": pending, "running": state.registry.tasks().running_all().len(), "instance": crate::instance::info(&state.cfg)})),
+        Json(json!({"status": if failures > 0 { "prewarm_failed" } else { "warming" }, "version": BUILD_SHA, "prewarm_pending": pending, "prewarm_failed": failures, "running": state.registry.tasks().running_all().len(), "instance": crate::instance::info(&state.cfg)})),
     )
         .into_response()
 }
diff --git a/crates/walgit-server/src/lfs.rs b/crates/walgit-server/src/lfs.rs
index d6b3f29..43e1d4c 100644
--- a/crates/walgit-server/src/lfs.rs
+++ b/crates/walgit-server/src/lfs.rs
@@ -82,7 +82,16 @@ pub async fn batch(
     if !st.cfg.lfs.enabled {
         return Err(ApiError::NotFound("lfs disabled".into()));
     }
-    let _ = st.auth.require_read(headers).await.map_err(auth_err)?;
+    if body.operation == "upload" {
+        let _ = st.auth.require_write(headers).await.map_err(auth_err)?;
+    } else if body.operation == "download" {
+        let _ = st.auth.require_read(headers).await.map_err(auth_err)?;
+    } else {
+        return Err(ApiError::BadRequest(format!(
+            "unsupported lfs operation: {}",
+            body.operation
+        )));
+    }
     not_served_here(st, &route.id)?;
     let handle = open_repo(st, &route.id, false).await?;
     let store = handle.store().clone();
diff --git a/crates/walgit-server/src/prewarm.rs b/crates/walgit-server/src/prewarm.rs
index cf14bf5..faa7191 100644
--- a/crates/walgit-server/src/prewarm.rs
+++ b/crates/walgit-server/src/prewarm.rs
@@ -2,8 +2,8 @@
 //! to this instance (packs when they fit, the remote pack indexes otherwise)
 //! and touch the default branch's root tree, so the first user request on a
 //! fresh instance finds everything in place. Each repo is a `prewarm` task
-//! (discoverable at `…/tasks`); `/readyz` can be gated on completion
-//! (`cache.prewarm_ready_timeout`).
+//! (discoverable at `…/tasks`); `/readyz` is ready only after every required
+//! prewarm succeeds.
 
 use std::sync::Arc;
 use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering};
@@ -13,9 +13,11 @@ use tracing::Instrument;
 use crate::AppState;
 
 pub struct Readiness {
-    /// All prewarms finished (ok or not).
+    /// All configured prewarms finished successfully.
     pub done: AtomicBool,
     pub pending: AtomicUsize,
+    /// Number of prewarm tasks that finished with an error in the current run.
+    pub failed: AtomicUsize,
     pub started_at: Instant,
 }
 
@@ -24,14 +26,14 @@ impl Readiness {
         Arc::new(Readiness {
             done: AtomicBool::new(true),
             pending: AtomicUsize::new(0),
+            failed: AtomicUsize::new(0),
             started_at: Instant::now(),
         })
     }
-    /// True when traffic may be routed here.
-    pub fn ready(&self, timeout: std::time::Duration) -> bool {
-        self.done.load(Ordering::Acquire)
-            || timeout.is_zero()
-            || self.started_at.elapsed() >= timeout
+    /// True only when every configured prewarm completed successfully.
+    ///
+    pub fn ready(&self) -> bool {
+        self.done.load(Ordering::Acquire) && self.failed.load(Ordering::Acquire) == 0
     }
 }
 
@@ -40,6 +42,7 @@ impl Default for Readiness {
         Readiness {
             done: AtomicBool::new(true),
             pending: AtomicUsize::new(0),
+            failed: AtomicUsize::new(0),
             started_at: Instant::now(),
         }
     }
@@ -52,6 +55,7 @@ pub fn spawn(state: Arc<AppState>) {
         return;
     }
     state.readiness.done.store(false, Ordering::Release);
+    state.readiness.failed.store(0, Ordering::Release);
     state
         .readiness
         .pending
@@ -68,7 +72,10 @@ pub fn spawn(state: Arc<AppState>) {
                 let t = Instant::now();
                 match warm(&st, &r).await {
                     Ok(summary) => tracing::info!(repo = %r, elapsed_ms = t.elapsed().as_millis() as u64, "prewarm: {summary}"),
-                    Err(e) => tracing::warn!(repo = %r, elapsed_ms = t.elapsed().as_millis() as u64, "prewarm failed: {e}"),
+                    Err(e) => {
+                        st.readiness.failed.fetch_add(1, Ordering::AcqRel);
+                        tracing::warn!(repo = %r, elapsed_ms = t.elapsed().as_millis() as u64, "prewarm failed: {e}");
+                    }
                 }
                 st.readiness.pending.fetch_sub(1, Ordering::AcqRel);
             }));
@@ -77,7 +84,15 @@ pub fn spawn(state: Arc<AppState>) {
             let _ = h.await;
         }
         state.readiness.done.store(true, Ordering::Release);
-        tracing::info!("prewarm complete; instance ready");
+        let failures = state.readiness.failed.load(Ordering::Acquire);
+        if failures == 0 {
+            tracing::info!("prewarm complete; instance ready");
+        } else {
+            tracing::error!(
+                failures,
+                "prewarm complete with failures; instance remains unready"
+            );
+        }
     });
 }
 
diff --git a/crates/walgit-server/src/web/v1.rs b/crates/walgit-server/src/web/v1.rs
index 49813f8..ec4f9b3 100644
--- a/crates/walgit-server/src/web/v1.rs
+++ b/crates/walgit-server/src/web/v1.rs
@@ -16,13 +16,13 @@ use std::sync::Arc;
 use axum::{
     Router,
     body::Body,
-    extract::{Path, Request, State},
+    extract::{Path, Query, Request, State},
     http::{HeaderMap, HeaderValue, Method, StatusCode, header},
     middleware::Next,
     response::{Html, IntoResponse, Response},
     routing::{get, post},
 };
-use serde::Serialize;
+use serde::{Deserialize, Serialize};
 
 use crate::repo::RepoRoute;
 use crate::web::api::{Need, RefInfo, etag_for, json_swr, run};
@@ -274,14 +274,31 @@ async fn me(State(st): State<Arc<AppState>>, headers: HeaderMap) -> Response {
     }
 }
 
+#[derive(Deserialize, Default)]
+struct AuthenticateQuery {
+    /// Origin of the SDK page that opened this popup. It is accepted only when
+    /// it is already allowed by the server's credentialed CORS policy.
+    origin: Option<String>,
+}
+
 /// `GET /api/v1/authenticate`: the popup landing page of the browser lane.
 /// `require_auth` sends an unauthenticated browser through sign-in first; this
 /// authenticated page then tells its opener and closes. The SDK opens it when a
 /// browser-lane call answers 401.
-async fn authenticate(State(st): State<Arc<AppState>>, headers: HeaderMap) -> Response {
+async fn authenticate(
+    State(st): State<Arc<AppState>>,
+    headers: HeaderMap,
+    Query(query): Query<AuthenticateQuery>,
+) -> Response {
     match st.auth.require_read(&headers).await {
         Ok(p) => {
-            let page = AUTHENTICATE_HTML.replace("{{principal}}", &html_escape(&p.name));
+            let target_origin = query
+                .origin
+                .filter(|origin| origin_allowed(&st.cfg, origin))
+                .unwrap_or_else(|| crate::smart::request_base_url(&st, &headers));
+            let page = AUTHENTICATE_HTML
+                .replace("{{principal}}", &html_escape(&p.name))
+                .replace("{{target_origin}}", &html_escape(&target_origin));
             let mut r = Html(page).into_response();
             r.headers_mut()
                 .insert(header::CACHE_CONTROL, HeaderValue::from_static("no-store"));
@@ -300,8 +317,8 @@ const AUTHENTICATE_HTML: &str = r#"<!doctype html>
 <script>
 (function () {
   var msg = { type: "repos:authenticated", principal: "{{principal}}" };
-  try { if (window.opener) { window.opener.postMessage(msg, "*"); } } catch (e) {}
-  try { if (window.parent && window.parent !== window) { window.parent.postMessage(msg, "*"); } } catch (e) {}
+  try { if (window.opener) { window.opener.postMessage(msg, "{{target_origin}}"); } } catch (e) {}
+  try { if (window.parent && window.parent !== window) { window.parent.postMessage(msg, "{{target_origin}}"); } } catch (e) {}
   // Only a window we opened ourselves (same site or cross-site via the SDK) is closed.
   if (window.opener) { setTimeout(function () { window.close(); }, 150); }
 })();
@@ -416,6 +433,19 @@ async fn repo_admin(
 mod tests {
     use super::*;
 
+    #[test]
+    fn popup_origin_must_match_configured_cors_origin() {
+        let mut cfg = walgit_config::Config::default();
+        cfg.server.cors_origins = vec![
+            "https://docs.example.com".into(),
+            "https://*.trusted.example.com".into(),
+        ];
+        assert!(origin_allowed(&cfg, "https://docs.example.com"));
+        assert!(origin_allowed(&cfg, "https://a.trusted.example.com"));
+        assert!(!origin_allowed(&cfg, "https://evil.example.com"));
+        assert!(!origin_allowed(&cfg, "https://trusted.example.com/path"));
+    }
+
     #[test]
     fn cors_covers_prefix_form_and_v1() {
         assert!(is_cors_api_path("/api/v1/me"));
diff --git a/crates/walgit-server/tests/sim.rs b/crates/walgit-server/tests/sim.rs
index 65e2026..e691ff3 100644
--- a/crates/walgit-server/tests/sim.rs
+++ b/crates/walgit-server/tests/sim.rs
@@ -1510,12 +1510,20 @@ async fn liveness_frozen_task_owner_does_not_wedge_readiness() -> Result<()> {
 
     let ready = walgit_server::prewarm::Readiness::new();
     ready.done.store(false, Ordering::Release);
-    ensure!(!ready.ready(Duration::from_millis(50)));
+    ensure!(!ready.ready());
     tokio::time::sleep(Duration::from_millis(60)).await;
     ensure!(
-        ready.ready(Duration::from_millis(50)),
-        "readiness joined a frozen prewarm forever"
+        !ready.ready(),
+        "readiness must remain false while prewarm is incomplete"
     );
+    ready.done.store(true, Ordering::Release);
+    ready.failed.store(1, Ordering::Release);
+    ensure!(
+        !ready.ready(),
+        "readiness must remain false after a failed prewarm"
+    );
+    ready.failed.store(0, Ordering::Release);
+    ensure!(ready.ready());
     Ok(())
 }
 
diff --git a/crates/walgit-store/src/gcs.rs b/crates/walgit-store/src/gcs.rs
index 071b63c..56e6234 100644
--- a/crates/walgit-store/src/gcs.rs
+++ b/crates/walgit-store/src/gcs.rs
@@ -585,12 +585,19 @@ impl GcsStore {
     }
 
     async fn put_inner(&self, key: &str, body: PutBody, opts: PutOptions) -> Result<ObjectMeta> {
+        if let PutMode::Update(version) = &opts.mode {
+            if parse_generation(version).is_none() {
+                return Err(StoreError::InvalidArgument(format!(
+                    "invalid GCS generation for conditional update on {key}: {version}"
+                )));
+            }
+        }
         let result = match body {
             PutBody::Bytes(b) => {
                 let (client, _permit) = self.data_client(key, false).await;
                 let mut builder =
                     client.write_object(self.bucket_resource.clone(), key.to_owned(), b);
-                builder = apply_put_opts(builder, &opts);
+                builder = apply_put_opts(builder, &opts)?;
                 builder.send_unbuffered().await
             }
             PutBody::File(path) => {
@@ -606,7 +613,7 @@ impl GcsStore {
                         key.to_owned(),
                         Bytes::from(bytes),
                     );
-                    builder = apply_put_opts(builder, &opts);
+                    builder = apply_put_opts(builder, &opts)?;
                     builder.send_unbuffered().await
                 } else {
                     let stream = crate::util::file_stream(path, None, FILE_CHUNK_SIZE);
@@ -616,7 +623,7 @@ impl GcsStore {
                     let (client, _permit) = self.data_client(key, false).await;
                     let mut builder =
                         client.write_object(self.bucket_resource.clone(), key.to_owned(), source);
-                    builder = apply_put_opts(builder, &opts);
+                    builder = apply_put_opts(builder, &opts)?;
                     builder.send_buffered().await
                 }
             }
@@ -625,7 +632,7 @@ impl GcsStore {
                 let (client, _permit) = self.data_client(key, false).await;
                 let mut builder =
                     client.write_object(self.bucket_resource.clone(), key.to_owned(), bytes);
-                builder = apply_put_opts(builder, &opts);
+                builder = apply_put_opts(builder, &opts)?;
                 builder.send_unbuffered().await
             }
             PutBody::Stream { stream, .. } => {
@@ -635,7 +642,7 @@ impl GcsStore {
                 let (client, _permit) = self.data_client(key, false).await;
                 let mut builder =
                     client.write_object(self.bucket_resource.clone(), key.to_owned(), source);
-                builder = apply_put_opts(builder, &opts);
+                builder = apply_put_opts(builder, &opts)?;
                 builder.send_buffered().await
             }
         };
@@ -1114,7 +1121,7 @@ impl StreamingSource for StoreStreamSource {
 fn apply_put_opts<T, S>(
     mut builder: google_cloud_storage::builder::storage::WriteObject<T, S>,
     opts: &PutOptions,
-) -> google_cloud_storage::builder::storage::WriteObject<T, S>
+) -> Result<google_cloud_storage::builder::storage::WriteObject<T, S>>
 where
     S: google_cloud_storage::stub::Storage + 'static,
 {
@@ -1124,9 +1131,12 @@ where
             builder = builder.set_if_generation_match(0_i64);
         }
         PutMode::Update(v) => {
-            if let Some(generation) = parse_generation(v) {
-                builder = builder.set_if_generation_match(generation);
-            }
+            let generation = parse_generation(v).ok_or_else(|| {
+                StoreError::InvalidArgument(format!(
+                    "invalid GCS generation for conditional update: {v}"
+                ))
+            })?;
+            builder = builder.set_if_generation_match(generation);
         }
     }
 
@@ -1138,7 +1148,7 @@ where
         builder = builder.set_cache_control(IMMUTABLE_CACHE_CONTROL);
     }
 
-    builder
+    Ok(builder)
 }
 
 // ---- error mapping ----
@@ -1240,6 +1250,12 @@ mod tests {
         assert_eq!(parse_generation(&v), None);
     }
 
+    #[test]
+    fn invalid_update_version_is_rejected_before_request_construction() {
+        let v = Version::new("etag-from-another-backend");
+        assert!(parse_generation(&v).is_none());
+    }
+
     #[test]
     fn range_to_read_range_basic() {
         let _r = range_to_read_range(&(10..30)); // [10, 30) → offset=10, limit=20
diff --git a/crates/walgit-store/src/s3.rs b/crates/walgit-store/src/s3.rs
index ac4359e..9417275 100644
--- a/crates/walgit-store/src/s3.rs
+++ b/crates/walgit-store/src/s3.rs
@@ -21,21 +21,18 @@
 //!
 //! ## Conditional DELETE
 //!
-//! S3 has no native conditional delete. We emulate via HEAD (read ETag) +
-//! compare + DELETE, documenting the inherent check-then-act race: a
-//! concurrent writer could replace the object between HEAD and DELETE.
-//! Acceptable for walgit's lease-guarded semantics.
+//! Conditional deletes use S3's native `If-Match` ETag precondition. The
+//! precondition is evaluated by S3 at the delete linearization point; callers
+//! must not emulate it with `HEAD` followed by an unconditional delete.
 //!
 //! ## Multipart upload
 //!
 //! Objects above `cfg.multipart_threshold` use CreateMultipartUpload +
-//! UploadPart + CompleteMultipartUpload. CreateMultipartUpload does NOT
-//! support `If-None-Match`/`If-Match` in the S3 API, so multipart is only
-//! used for `PutMode::Overwrite`. For walgit's immutable pack objects
-//! (`PutMode::Create`) we use single-shot PUT when the object is large,
-//! accepting the (tiny) risk of concurrent create races. CAS-rewritten
-//! objects (manifests, leases, bundle lists) are always small → single-shot
-//! PUT with conditional headers.
+//! UploadPart + CompleteMultipartUpload. S3 applies `If-None-Match`/`If-Match`
+//! at multipart completion, so immutable creates and CAS updates remain
+//! conditional even when the object is larger than the single-PUT limit.
+//! Completion conflicts are surfaced as precondition failures and incomplete
+//! uploads are aborted on every failure path.
 //!
 //! ## rustfs compatibility (tested with rustfs/rustfs:latest)
 //!
@@ -185,7 +182,7 @@ impl S3Store {
             404 => Err(StoreError::NotFound { key: key.into() }),
             412 => Err(StoreError::PreconditionFailed {
                 key: key.into(),
-                current: etag.map(|e| Version::new(e)),
+                current: etag.map(Version::new),
             }),
             s if s >= 500 || s == 429 => {
                 Err(StoreError::Retryable(anyhow::anyhow!("s3 get status {s}")))
@@ -233,7 +230,7 @@ fn err_code<E>(err: &aws_sdk_s3::error::SdkError<E>) -> Option<&str>
 where
     E: aws_sdk_s3::error::ProvideErrorMetadata,
 {
-    err.as_service_error().map(|e| e.meta().code()).flatten()
+    err.as_service_error().and_then(|e| e.meta().code())
 }
 
 fn classify_put_error(
@@ -300,11 +297,9 @@ impl ObjectStore for S3Store {
     async fn put(&self, key: &str, body: PutBody, opts: PutOptions) -> Result<ObjectMeta> {
         let (s3_body, len) = body_to_s3(body).await?;
 
-        // Multipart only for Overwrite (CreateMultipartUpload has no
-        // conditional header support in the S3 API). Create/Update always
-        // use single-shot PUT.
-        let use_multipart =
-            len > self.multipart_threshold && matches!(opts.mode, PutMode::Overwrite);
+        // Apply conditional create/update at multipart completion. This keeps
+        // the storage abstraction atomic for large immutable packs too.
+        let use_multipart = len > self.multipart_threshold;
 
         if use_multipart {
             return self.multipart_put(key, s3_body, len, &opts).await;
@@ -345,10 +340,10 @@ impl ObjectStore for S3Store {
             Err(e) => {
                 let mut err = classify_put_error(key, e);
                 // Fill `current` via HEAD if we got a PreconditionFailed.
-                if let StoreError::PreconditionFailed { current: c, .. } = &mut err {
-                    if c.is_none() {
-                        *c = self.head(key).await.ok().flatten().map(|m| m.version);
-                    }
+                if let StoreError::PreconditionFailed { current: c, .. } = &mut err
+                    && c.is_none()
+                {
+                    *c = self.head(key).await.ok().flatten().map(|m| m.version);
                 }
                 Err(err)
             }
@@ -356,34 +351,24 @@ impl ObjectStore for S3Store {
     }
 
     async fn delete(&self, key: &str, if_version: Option<Version>) -> Result<()> {
+        let mut request = self.client.delete_object().bucket(&self.bucket).key(key);
         if let Some(want) = &if_version {
-            // S3 has no conditional delete: emulate via HEAD + compare + DELETE.
-            // RACE: a concurrent writer could replace the object between HEAD
-            // and DELETE. Acceptable for walgit's lease-guarded semantics.
-            let head = self.head(key).await?;
-            match head {
-                None => return Err(StoreError::NotFound { key: key.into() }),
-                Some(meta) if &meta.version != want => {
-                    return Err(StoreError::PreconditionFailed {
-                        key: key.into(),
-                        current: Some(meta.version),
-                    });
-                }
-                _ => {}
-            }
+            request = request.if_match(want.as_str());
         }
-
-        let resp = self
-            .client
-            .delete_object()
-            .bucket(&self.bucket)
-            .key(key)
-            .send()
-            .await;
+        let resp = request.send().await;
 
         match resp {
             Ok(_) => Ok(()),
             Err(err) => {
+                let code = err_code(&err).unwrap_or("");
+                if if_version.is_some()
+                    && matches!(code, "PreconditionFailed" | "ConditionalRequestConflict")
+                {
+                    return Err(StoreError::PreconditionFailed {
+                        key: key.into(),
+                        current: self.head(key).await.ok().flatten().map(|m| m.version),
+                    });
+                }
                 // S3 DeleteObject is idempotent: deleting a non-existent key
                 // returns Ok, not an error. If we get here, it's a real error.
                 // For unconditional deletes we treat any error as transient.
@@ -547,14 +532,6 @@ impl ObjectStore for S3Store {
                 "compose needs at least one source".into(),
             ));
         }
-        if let PutMode::Create = opts.mode
-            && self.head(dest).await?.is_some()
-        {
-            return Err(StoreError::PreconditionFailed {
-                key: dest.to_owned(),
-                current: None,
-            });
-        }
         // Sizes first: the layout of parts depends on them.
         let mut sizes = Vec::with_capacity(sources.len());
         for src in sources {
@@ -697,19 +674,30 @@ impl ObjectStore for S3Store {
         let completed = aws_sdk_s3::types::CompletedMultipartUpload::builder()
             .set_parts(Some(parts))
             .build();
-        let resp = match self
+        let mut complete = self
             .client
             .complete_multipart_upload()
             .bucket(&self.bucket)
             .key(dest)
             .upload_id(&upload_id)
-            .multipart_upload(completed)
-            .send()
-            .await
-        {
+            .multipart_upload(completed);
+        if let PutMode::Create = &opts.mode {
+            complete = complete.if_none_match("*");
+        }
+        if let PutMode::Update(v) = &opts.mode {
+            complete = complete.if_match(v.as_str());
+        }
+        let resp = match complete.send().await {
             Ok(r) => r,
             Err(e) => {
                 let _ = self.abort_multipart(dest, &upload_id).await;
+                let code = err_code(&e).unwrap_or("");
+                if matches!(code, "PreconditionFailed" | "ConditionalRequestConflict") {
+                    return Err(StoreError::PreconditionFailed {
+                        key: dest.to_owned(),
+                        current: self.head(dest).await.ok().flatten().map(|m| m.version),
+                    });
+                }
                 return Err(StoreError::Other(anyhow::anyhow!(
                     "s3 complete multipart: {e}"
                 )));
@@ -749,7 +737,7 @@ struct ListState {
     buffer: std::vec::IntoIter<Result<ObjectMeta>>,
 }
 
-// ---- multipart upload (Overwrite only) ---------------------------------
+// ---- multipart upload ----------------------------------------------------
 
 impl S3Store {
     async fn multipart_put(
@@ -900,7 +888,8 @@ impl S3Store {
 // 5. ListObjectsV2: StartAfter, ContinuationToken, IsTruncated/NextToken OK.
 // 6. DeleteObject: idempotent for absent keys (204).
 // 7. Multipart: CreateMultipartUpload + UploadPart + CompleteMultipartUpload
-//    supported. No conditional headers on Create/Complete (same as real S3).
+//    supported. Conditional Create/Update preconditions are applied at
+//    CompleteMultipartUpload; providers that reject them must fail closed.
 // 8. ETags: quoted, MD5 for single-PUT, compound for multipart. Quotes
 //    stripped consistently in our Version.
 // 9. force_path_style: required for rustfs local dev.
diff --git a/web/API.md b/web/API.md
index 6ca5381..afefff9 100644
--- a/web/API.md
+++ b/web/API.md
@@ -40,7 +40,9 @@ pre-D15 `/services/api/{owner}/{repo}/…` are gone (prototyping phase — no al
 Every request authenticates (§1.3 of AGENTS.md); a missing/expired browser
 session answers `401` and the SDK opens **`/api-browser/v1/authenticate`** in a popup —
 walgit's own sign-in (`/_auth/login`) runs before the application page `postMessage`s
-`{type: "repos:authenticated"}` to its opener and closes — then retries once.
+`{type: "repos:authenticated"}` to the validated opener origin and closes — then retries once. The SDK passes
+its page origin as a query parameter; the server accepts it only when it is in `server.cors_origins`, otherwise
+it falls back to the walgit host origin.
 `GET /api/v1/me` → `{principal, write, anonymous}` (`no-store`).
 
 CORS: origins listed in `server.cors_origins` (exact, or one leading `*.`,
diff --git a/web/sdk/README.md b/web/sdk/README.md
index 180b179..d1d86d1 100644
--- a/web/sdk/README.md
+++ b/web/sdk/README.md
@@ -23,7 +23,7 @@ SDK picks the lane, handles the sign-in popup, and unwraps long answers.
 
 | You are… | Lane | What the SDK does |
 |---|---|---|
-| a page on another origin listed in `server.cors_origins` | browser `/{owner}/{repo}/api-browser/*` | `fetch` with `credentials: "include"`. On `401` it opens `<host>/api-browser/v1/authenticate`; walgit's sign-in (`/_auth/login`, your OIDC issuer) runs, the landing page posts `repos:authenticated`, and the SDK retries once. |
+| a page on another origin listed in `server.cors_origins` | browser `/{owner}/{repo}/api-browser/*` | `fetch` with `credentials: "include"`. On `401` it opens `<host>/api-browser/v1/authenticate?origin=<page-origin>`; walgit's sign-in (`/_auth/login`, your OIDC issuer) runs, the landing page posts `repos:authenticated` only to the validated opener origin, and the SDK retries once. |
 | the bundled UI on git.example.com | same-origin `/{owner}/{repo}/api/*` | the session cookie; a lapsed session is sent to `/_auth/login` |
 | a script, agent, CI job, Node | bearer `/{owner}/{repo}/api/*` | `createClient({ token })` with a walgit access token (`/_auth/tokens`), a static token, or an ID token → `Authorization: Bearer` |
 
diff --git a/web/sdk/repos.ts b/web/sdk/repos.ts
index 3b90b4e..90ab2ef 100644
--- a/web/sdk/repos.ts
+++ b/web/sdk/repos.ts
@@ -353,7 +353,7 @@ export class ReposClient {
 
   private async openAuthPopup(): Promise<boolean> {
     if (typeof window === "undefined") return false;
-    const url = `${this.base}/api-browser/v1/authenticate`;
+    const url = `${this.base}/api-browser/v1/authenticate?origin=${encodeURIComponent(window.location.origin)}`;
     const w = 520;
     const h = 640;
     const left = Math.max(0, (window.screen?.width ?? w) / 2 - w / 2);