From 393302f568919efc51c3cb455bafd6ad0772c346 Mon Sep 17 00:00:00 2001 From: kangmaup Date: Thu, 24 Sep 2026 13:45:40 +0700 Subject: [PATCH 01/13] test(rustasea): align the livewire manifest assertion with the scaffold `artisan_alias_generates_livewire_app` still expected the generated `Cargo.toml` to enable `features = ["view", "broadcast"]`. The scaffold has emitted `["view", "action"]` since ab02ac6 (its own `manifest.rs` unit test asserts exactly that), and the umbrella crate has no `broadcast` feature, so the old expectation could never pass and kept the CI test job red. --- crates/rustasea/tests/cli.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/rustasea/tests/cli.rs b/crates/rustasea/tests/cli.rs index b1d8164..010ab4d 100644 --- a/crates/rustasea/tests/cli.rs +++ b/crates/rustasea/tests/cli.rs @@ -143,7 +143,7 @@ fn artisan_alias_generates_livewire_app() { let app = dir.join("demo-app"); assert!(app.join("resources/views/partials/counter.html").exists()); let manifest = std::fs::read_to_string(app.join("Cargo.toml")).expect("read manifest"); - assert!(manifest.contains("features = [\"view\", \"broadcast\"]")); + assert!(manifest.contains("features = [\"view\", \"action\"]")); std::fs::remove_dir_all(&dir).ok(); } From a7a3cdba8047b6b02be76559b1e4c38c0e307bdd Mon Sep 17 00:00:00 2001 From: kangmaup Date: Thu, 24 Sep 2026 13:49:06 +0700 Subject: [PATCH 02/13] test(app): give each two-factor login its own throttle bucket `enable_confirm_challenge_and_recovery_lifecycle` failed with `429` on every full run. The `login` limiter allows five attempts per minute per `username|ip`, and its registry is a process-wide static. Test requests carry no `ConnectInfo`, so every login resolves to the `0.0.0.0` peer: the lifecycle test logs in as `ada@example.com` five times, and `auth_log` logs in as the same user through the same limiter within the same minute. `start_challenge` now attaches a distinct loopback `ConnectInfo` to each login, so the throttle no longer couples otherwise independent tests. Production code is unchanged. --- .../src/routes/tests/two_factor.rs | 24 ++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/crates/rustasea-app/src/routes/tests/two_factor.rs b/crates/rustasea-app/src/routes/tests/two_factor.rs index a75310c..df25a8a 100644 --- a/crates/rustasea-app/src/routes/tests/two_factor.rs +++ b/crates/rustasea-app/src/routes/tests/two_factor.rs @@ -10,11 +10,13 @@ //! [`MemoryUserProvider`]: rustasea::auth::MemoryUserProvider //! [`PROVIDER_LOCK`]: super::settings_flows::PROVIDER_LOCK +use std::net::SocketAddr; +use std::sync::atomic::{AtomicU8, Ordering}; use std::sync::Arc; use std::time::{SystemTime, UNIX_EPOCH}; use axum::body::Body; -use axum::extract::Extension; +use axum::extract::{ConnectInfo, Extension}; use axum::http::{header, HeaderMap, HeaderValue, Request, StatusCode}; use axum::Router; use rustasea::auth::users::{AuthUserRecord, MemoryUserRegistry}; @@ -227,11 +229,27 @@ async fn confirm(secret: &str) { assert_eq!(status, StatusCode::OK, "confirm: {body}"); } +/// Last octet of the peer address handed to the next login (see [`unique_peer`]). +static NEXT_PEER_OCTET: AtomicU8 = AtomicU8::new(1); + +/// A distinct loopback peer for one login request. +/// +/// The `login` limiter allows five attempts per minute per `username|ip`, and +/// its registry is process-wide. Without `ConnectInfo` every test login +/// resolves to the same `0.0.0.0` peer, so the lifecycle test's five logins as +/// `ada@example.com`, on top of other modules' logins as the same user, hit +/// `429`. A fresh peer per login keeps each request in its own bucket. +fn unique_peer() -> ConnectInfo { + let octet = NEXT_PEER_OCTET.fetch_add(1, Ordering::Relaxed); + ConnectInfo(SocketAddr::from(([127, 0, 2, octet], 54321))) +} + /// Start a challenge by logging in; returns the pending session id. async fn start_challenge(guard: &Arc) -> String { let body = format!("email={USER_A_EMAIL}&password={USER_A_PASSWORD}"); - let (status, headers, response) = - call(app_with_guard(guard.clone()), post("/login", &body)).await; + let mut request = post("/login", &body); + request.extensions_mut().insert(unique_peer()); + let (status, headers, response) = call(app_with_guard(guard.clone()), request).await; assert_eq!(status, StatusCode::FOUND, "login: {response}"); assert_eq!(location(&headers).as_deref(), Some("/two-factor-challenge")); cookie_value(&headers, SESSION_COOKIE_NAME).expect("pending session cookie") From c48dd2353f2bf128b2b72d51100a64853af5d10f Mon Sep 17 00:00:00 2001 From: kangmaup Date: Thu, 24 Sep 2026 13:50:14 +0700 Subject: [PATCH 03/13] test(queue-dashboard): serialize the tests that share the heartbeat registry `workers_renders_heartbeats` and `workers_flags_stale_heartbeats_inactive` both clear, stamp, and read `rustasea_queue::heartbeat`'s process-wide registry. Run in parallel, one test's `clear`/`stamp_at` lands in the middle of the other's snapshot (`left: 2, right: 1`), which failed the CI test job on every run. A test-only lock now serializes the two. --- crates/rustasea-queue-dashboard/src/lib.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/crates/rustasea-queue-dashboard/src/lib.rs b/crates/rustasea-queue-dashboard/src/lib.rs index 05e56c8..ec773ed 100644 --- a/crates/rustasea-queue-dashboard/src/lib.rs +++ b/crates/rustasea-queue-dashboard/src/lib.rs @@ -387,9 +387,17 @@ mod tests { let _ = std::fs::remove_dir_all(&dir); } + /// Serializes the tests that reset and read the process-wide heartbeat + /// registry: run in parallel, one test's `clear`/`stamp_at` lands in the + /// middle of the other's snapshot. + static HEARTBEATS_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(()); + /// `workers` renders the heartbeat registry as ordered, active rows. #[test] fn workers_renders_heartbeats() { + let _serial = HEARTBEATS_LOCK + .lock() + .unwrap_or_else(|poison| poison.into_inner()); heartbeat::clear(); let at = Utc .timestamp_opt(1_700_000_000, 0) @@ -406,6 +414,9 @@ mod tests { /// A stale heartbeat is reported but flagged `active == false`. #[test] fn workers_flags_stale_heartbeats_inactive() { + let _serial = HEARTBEATS_LOCK + .lock() + .unwrap_or_else(|poison| poison.into_inner()); heartbeat::clear(); let now = Utc::now(); heartbeat::stamp_at("qd-fresh", now); From ac9068b0bef185229631f1a08defa826ca4951c3 Mon Sep 17 00:00:00 2001 From: kangmaup Date: Thu, 24 Sep 2026 13:51:22 +0700 Subject: [PATCH 04/13] ci: only route rustc through sccache where it is installed The `cargo-deny` job failed on every run: the workflow set `RUSTC_WRAPPER: sccache` for all jobs, but only `quality`, `test`, and `msrv` install sccache. `cargo deny` runs `cargo metadata`, which invokes `rustc -vV` through the wrapper and fails with "could not execute process `sccache ...`". `cargo audit` passed because it only reads `Cargo.lock`. `RUSTC_WRAPPER` and `SCCACHE_GHA_ENABLED` now live on the three jobs that install sccache. `.cargo/config.toml` also drops its `rustc-wrapper = "/home/vheins/.cargo/bin/sccache"`: that path exists on a single machine, so every other checkout failed its first cargo command, and the `deny` job would have fallen back to it once the env var was gone. Local sccache is now opt-in, as documented in CONTRIBUTING. --- .cargo/config.toml | 9 +++++---- .github/workflows/ci.yml | 20 +++++++++++++++----- CONTRIBUTING.md | 2 +- 3 files changed, 21 insertions(+), 10 deletions(-) diff --git a/.cargo/config.toml b/.cargo/config.toml index ce2708a..7a37a32 100644 --- a/.cargo/config.toml +++ b/.cargo/config.toml @@ -1,6 +1,7 @@ -[build] -rustc-wrapper = "/home/vheins/.cargo/bin/sccache" - +# sccache is opt-in: export `RUSTC_WRAPPER=sccache` (or set +# `build.rustc-wrapper` in your own `~/.cargo/config.toml`) to route rustc +# through it. A machine-specific wrapper path here would break every other +# checkout; CI sets `RUSTC_WRAPPER` only in the jobs that install sccache. [env] SCCACHE_CACHE_SIZE = "5G" CARGO_INCREMENTAL = "0" @@ -8,4 +9,4 @@ CARGO_INCREMENTAL = "0" # Convenience alias so the documented `cargo xtask ` surface works without # a separate `cargo-xtask` binary (ADOPT-030). Expands to `cargo run -p xtask --`. [alias] -xtask = "run -p xtask --" \ No newline at end of file +xtask = "run -p xtask --" diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7d16291..10987ab 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -22,11 +22,12 @@ concurrency: env: CARGO_TERM_COLOR: always - # Route rustc through sccache and store the cache in the GitHub Actions - # cache. Set workflow-wide: the compile-free `deny`/`audit` jobs never - # invoke rustc, so the wrapper is inert there (no sccache binary installed). - RUSTC_WRAPPER: sccache - SCCACHE_GHA_ENABLED: "true" + +# `RUSTC_WRAPPER: sccache` routes rustc through sccache and stores the cache in +# the GitHub Actions cache. It is set per job, only where +# `mozilla-actions/sccache-action` installs the binary: `cargo deny` runs +# `cargo metadata`, which invokes `rustc -vV`, so a wrapper without the binary +# fails the `deny` job. jobs: # fmt + clippy (-D warnings) + workspace DAG cycle check. @@ -34,6 +35,9 @@ jobs: name: Format, lint & cycles runs-on: ubuntu-latest timeout-minutes: 15 + env: + RUSTC_WRAPPER: sccache + SCCACHE_GHA_ENABLED: "true" steps: - uses: actions/checkout@v4 - name: Install stable toolchain (rustfmt + clippy) @@ -53,6 +57,9 @@ jobs: name: Test (workspace) runs-on: ubuntu-latest timeout-minutes: 30 + env: + RUSTC_WRAPPER: sccache + SCCACHE_GHA_ENABLED: "true" steps: - uses: actions/checkout@v4 - name: Install stable toolchain @@ -100,6 +107,9 @@ jobs: name: MSRV (1.88.0) runs-on: ubuntu-latest timeout-minutes: 15 + env: + RUSTC_WRAPPER: sccache + SCCACHE_GHA_ENABLED: "true" steps: - uses: actions/checkout@v4 - name: Install Rust 1.88.0 diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 2e73f59..6e690bf 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -133,7 +133,7 @@ cargo test -p rustasea --features integration -- --ignored Formatting violations fail the build: run `cargo fmt --all` before pushing. - CI uses [sccache](https://github.com/mozilla/sccache) (`mozilla-actions/sccache-action`) to cache Rust compilation across the `quality`, `test`, and `msrv` jobs. + CI uses [sccache](https://github.com/mozilla/sccache) (`mozilla-actions/sccache-action`) to cache Rust compilation across the `quality`, `test`, and `msrv` jobs. Locally it is opt-in: install sccache and export `RUSTC_WRAPPER=sccache` (or set `build.rustc-wrapper` in your own `~/.cargo/config.toml`). 4. Address review feedback with new commits; avoid rewriting shared history. ## Coding standards From 85f80a8a0164dc043149a6d0d3205b04a12ddccd Mon Sep 17 00:00:00 2001 From: kangmaup Date: Thu, 24 Sep 2026 13:51:35 +0700 Subject: [PATCH 05/13] ci: request the reviewer from pull_request_target so fork PRs work `Assign vheins as reviewer` failed on every PR opened from a fork with "Resource not accessible by integration": a `pull_request` run from a fork only gets a read-only `GITHUB_TOKEN`, whatever the workflow's `permissions:` block asks for, so `requestReviewers` is refused. `pull_request_target` runs in the base repository's context with the declared `pull-requests: write` permission. It is safe for this workflow because the job never checks out or executes the PR's code; it only calls the REST API. GitHub reads `pull_request_target` workflows from the base branch, so the fix takes effect once it is merged. --- .github/workflows/auto-assign-reviewer.yml | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/.github/workflows/auto-assign-reviewer.yml b/.github/workflows/auto-assign-reviewer.yml index 8abc9c7..9dbc1b3 100644 --- a/.github/workflows/auto-assign-reviewer.yml +++ b/.github/workflows/auto-assign-reviewer.yml @@ -1,8 +1,14 @@ name: Auto Assign Reviewer # Requests a review from vheins on every PR that targets master. +# +# `pull_request_target` runs in the base repository's context, so the token can +# request reviewers on PRs opened from forks; a `pull_request` run from a fork +# only gets a read-only token and fails with "Resource not accessible by +# integration". This is safe here because the job never checks out or runs the +# PR's code: it only calls the REST API. on: - pull_request: + pull_request_target: types: [opened, ready_for_review, reopened, synchronize] branches: [master] From b6dd5c34050d0c8ef44f35fd2866ac49227805eb Mon Sep 17 00:00:00 2001 From: kangmaup Date: Thu, 24 Sep 2026 14:30:33 +0700 Subject: [PATCH 06/13] test(app): load the workspace app config in the tinker test `config_renders_json` loaded the process-relative `config/app`, but tests run with the crate root as their working directory, so the file was never found and `app_env` could only come from `APP_ENV`. The test passed only when a `bootstrap::providers` test happened to have `APP_ENV` set at that moment: usually true with many test threads, rarely true on the 4-core CI runners, where it failed the test job. It also fails when run on its own. The helper now locates the workspace `config/app.toml` from `CARGO_MANIFEST_DIR`, the same approach as `routes::resources_root`. --- crates/rustasea-app/src/bootstrap/tinker.rs | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/crates/rustasea-app/src/bootstrap/tinker.rs b/crates/rustasea-app/src/bootstrap/tinker.rs index 314cb47..f01ddbb 100644 --- a/crates/rustasea-app/src/bootstrap/tinker.rs +++ b/crates/rustasea-app/src/bootstrap/tinker.rs @@ -121,8 +121,14 @@ mod tests { use rustasea::foundation::CONFIG_LOADER_KEY; /// Build a container with a config loader + environment binding. + /// + /// Tests run with the crate root as their working directory, so the + /// process-relative `config/app` the app loads at runtime is never found + /// here. The workspace `config/app.toml` is located from + /// `CARGO_MANIFEST_DIR` instead (the crate lives two levels below it). fn container() -> (Arc, Container) { - let loader = Arc::new(ConfigLoader::load_from(&["config/app"]).expect("load config")); + let app_config = concat!(env!("CARGO_MANIFEST_DIR"), "/../../config/app"); + let loader = Arc::new(ConfigLoader::load_from(&[app_config]).expect("load config")); let mut container = Container::new(); container.instance(CONFIG_LOADER_KEY, Arc::clone(&loader)); container.instance("app.environment", "local".to_string()); @@ -134,8 +140,8 @@ mod tests { fn config_renders_json() { let (loader, container) = container(); let source = AppTinkerSource::from_booted(loader, "local".to_string(), &container); - // `app_env` is always resolvable (defaulted) — it must render as a JSON - // string, never panic. + // `app_env` comes from `config/app.toml` (an `APP_ENV` set by a + // concurrent test only overrides it) and must render as a JSON string. let rendered = source.config("app_env").expect("app_env resolves"); assert!( rendered.starts_with('"'), From f577c9f77c4cb830ceb9bd488731204b4b656263 Mon Sep 17 00:00:00 2001 From: kangmaup Date: Thu, 24 Sep 2026 14:44:39 +0700 Subject: [PATCH 07/13] ci: fetch every locked crate before running the tests `generated_app_compiles_for_every_variant` generates each starter kit and runs `cargo check --offline` on it. The react, vue, and svelte kits depend on WASM-side crates (`async-tungstenite`, `any_spawner`, `gloo-net`, ...) that the host `cargo test` build never downloads, so on a fresh runner the offline check failed with "attempting to make an HTTP request, but --offline was specified". Developer machines rarely notice because their cargo cache is already warm. A `cargo fetch` step now downloads every crate in `Cargo.lock`, for every target, before the tests run. Reproduced locally by moving those three crates out of the cargo cache (the gate fails exactly as on CI) and fixed by `cargo fetch`, which re-downloads them from the lockfile. --- .github/workflows/ci.yml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 10987ab..fc6d278 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -67,6 +67,12 @@ jobs: - name: Run sccache-cache uses: mozilla-actions/sccache-action@v0.0.11 - uses: Swatinem/rust-cache@v2 + # The scaffold compile gate (`crates/rustasea-scaffold/tests/generated_compiles.rs`) + # runs `cargo check --offline` on every generated starter kit, and the + # WASM variants need crates that the host build never downloads. Fetch + # every locked crate, for every target, before the tests run. + - name: cargo fetch + run: cargo fetch - name: cargo test run: cargo test --workspace From e5d1daf064fb2856bd1a3fcf030a112ef20c2b18 Mon Sep 17 00:00:00 2001 From: kangmaup Date: Thu, 24 Sep 2026 14:44:58 +0700 Subject: [PATCH 08/13] ci: report every failing test in one run Without `--no-fail-fast`, `cargo test` stops at the first failing test binary, so each broken test hid the ones behind it: fixing the scaffold assertion revealed the two-factor test, fixing that revealed the tinker and scaffold compile-gate failures, one CI round trip at a time. Running the whole suite on every push reports all failures at once. --- .github/workflows/ci.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index fc6d278..074e8c5 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -73,8 +73,10 @@ jobs: # every locked crate, for every target, before the tests run. - name: cargo fetch run: cargo fetch + # `--no-fail-fast` reports every failing test in one run instead of + # stopping at the first failing test binary. - name: cargo test - run: cargo test --workspace + run: cargo test --workspace --no-fail-fast # Supply-chain gate: license allow-list, advisory database, banned crates and # source provenance (configuration in deny.toml). From ddb9348d27d87f334303ef83e30d1336cdc4da95 Mon Sep 17 00:00:00 2001 From: kangmaup Date: Thu, 24 Sep 2026 14:59:47 +0700 Subject: [PATCH 09/13] test(orm): serialize the tests that share the query recorder slot `track_records_success_with_original_sql` failed intermittently on CI (`left: 0, right: 1`; about one run in 40 locally with 4 test threads). The three `profile` tests share the process-wide `RECORDER` slot, and `track_without_recorder_passes_through` or `recorder_registry_round_trips` could clear or replace the capturing recorder in the middle of its `track` call. A test-only tokio mutex now serializes the three (tokio, so the async tests can hold it across `.await`), and the capturing test only counts events for its own unique SQL, so a query tracked by a concurrent DB test cannot be mistaken for its event. 200 consecutive runs pass. --- crates/rustasea-orm/src/profile.rs | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/crates/rustasea-orm/src/profile.rs b/crates/rustasea-orm/src/profile.rs index a6309fa..d7ba1fa 100644 --- a/crates/rustasea-orm/src/profile.rs +++ b/crates/rustasea-orm/src/profile.rs @@ -116,6 +116,12 @@ pub async fn track( mod tests { use super::*; + /// Serializes the tests that install, clear, or rely on the process-wide + /// recorder slot: run in parallel, one test's `clear_query_recorder` or + /// `register_query_recorder` lands in the middle of another's `track` call. + /// A tokio mutex, so the async tests can hold it across `.await`. + static RECORDER_LOCK: tokio::sync::Mutex<()> = tokio::sync::Mutex::const_new(()); + /// Verifies the registry round-trips an installed recorder. #[test] fn recorder_registry_round_trips() { @@ -123,6 +129,7 @@ mod tests { impl QueryRecorder for Noop { fn record(&self, _event: SqlQueryEvent) {} } + let _serial = RECORDER_LOCK.blocking_lock(); clear_query_recorder(); assert!(query_recorder().is_none()); register_query_recorder(Arc::new(Noop)); @@ -146,9 +153,14 @@ mod tests { } } + // Unique SQL, so a query tracked by a concurrent DB test while this + // recorder is installed cannot be mistaken for this test's event. + const SQL: &str = "SELECT $1 -- profile::track_records_success_with_original_sql"; + + let _serial = RECORDER_LOCK.lock().await; let recorder = Arc::new(Capturing::default()); register_query_recorder(recorder.clone()); - let value = track("fetch_json", "SELECT $1", async { + let value = track("fetch_json", SQL, async { Ok::<_, crate::error::OrmError>(7) }) .await @@ -157,15 +169,16 @@ mod tests { assert_eq!(value, 7); let events = recorder.events.lock().unwrap(); - assert_eq!(events.len(), 1); - assert_eq!(events[0].kind, "fetch_json"); - assert_eq!(events[0].sql, "SELECT $1"); - assert!(events[0].error.is_none()); + let ours: Vec<&SqlQueryEvent> = events.iter().filter(|event| event.sql == SQL).collect(); + assert_eq!(ours.len(), 1); + assert_eq!(ours[0].kind, "fetch_json"); + assert!(ours[0].error.is_none()); } /// Verifies `track` runs untouched (and records nothing) with no recorder. #[tokio::test] async fn track_without_recorder_passes_through() { + let _serial = RECORDER_LOCK.lock().await; clear_query_recorder(); let value = track("execute_bind", "DELETE FROM t", async { Ok::<_, crate::error::OrmError>(3) From cc1519f8d7ee643094f1b18db22cc4236e9c5c8f Mon Sep 17 00:00:00 2001 From: Muhammad Rheza Alfin Date: Thu, 24 Sep 2026 16:34:23 +0700 Subject: [PATCH 10/13] review(pr-#3): request changes - Domain: CI workflows (sccache/cargo-deny, cargo fetch, --no-fail-fast), auth throttle ConnectInfo, test serialization (ORM recorder lock, queue heartbeat lock), scaffold manifest features - Findings: 1 HIGH, 1 LOW - Blocker: production serve path never attaches ConnectInfo; login throttle key is username|0.0.0.0 From 7d0eb244b3ecc10a488afc1cd8b6a72b6ae2fa45 Mon Sep 17 00:00:00 2001 From: kangmaup Date: Thu, 24 Sep 2026 17:09:48 +0700 Subject: [PATCH 11/13] fix(app): serve with ConnectInfo so the login throttle sees the client IP `main` served the router with `axum::serve(listener, router)`, which attaches no `ConnectInfo`, so every request's peer resolved to the `0.0.0.0` fallback. The `login` limiter keyed every client as `|0.0.0.0` (five failed attempts from anywhere locked a username out), `trusted_proxies` could never engage, and the authentication log recorded `0.0.0.0` for everyone. Serving moves into `serve()`, which uses `into_make_service_with_connect_info::()`. Regression tests: - `served_app_logs_the_connection_peer` logs in over a real TCP connection through `serve()` and asserts the audit row records `127.0.0.1`; it failed with `0.0.0.0` before this change; - `lockout_is_scoped_to_the_client_peer` locks one client out after five failed attempts while another client still logs in as the same user; - `successful_login_records_row` now asserts a non-loopback peer reaches the audit row instead of encoding the `0.0.0.0` fallback. --- crates/rustasea-app/src/main.rs | 23 +++- .../rustasea-app/src/routes/tests/auth_log.rs | 107 +++++++++++++++++- 2 files changed, 125 insertions(+), 5 deletions(-) diff --git a/crates/rustasea-app/src/main.rs b/crates/rustasea-app/src/main.rs index 1039cf5..cecee77 100644 --- a/crates/rustasea-app/src/main.rs +++ b/crates/rustasea-app/src/main.rs @@ -53,12 +53,29 @@ async fn main() -> anyhow::Result<()> { let listener = tokio::net::TcpListener::bind(addr).await?; println!("RustaSea dev server listening on http://{addr}"); - axum::serve(listener, router) - .with_graceful_shutdown(app.shutdown()) - .await?; + serve(listener, router, app.shutdown()).await?; Ok(()) } +/// Serve `router` on `listener` until `shutdown` resolves. +/// +/// Every request carries the connection's peer address as +/// `ConnectInfo`: the login throttle and the authentication log key +/// on the client IP, and without it every client falls back to `0.0.0.0` and +/// shares one throttle bucket. +pub(crate) async fn serve( + listener: tokio::net::TcpListener, + router: axum::Router, + shutdown: impl std::future::Future + Send + 'static, +) -> std::io::Result<()> { + axum::serve( + listener, + router.into_make_service_with_connect_info::(), + ) + .with_graceful_shutdown(shutdown) + .await +} + /// Resolve the bind address from `APP_URL` (host:port) or the default. fn bind_address() -> SocketAddr { std::env::var("APP_URL") diff --git a/crates/rustasea-app/src/routes/tests/auth_log.rs b/crates/rustasea-app/src/routes/tests/auth_log.rs index 3df0c89..f9bfbd3 100644 --- a/crates/rustasea-app/src/routes/tests/auth_log.rs +++ b/crates/rustasea-app/src/routes/tests/auth_log.rs @@ -21,9 +21,11 @@ //! [`AuthenticationLogLogger`]: rustasea_authlog::AuthenticationLogLogger //! [`PROVIDER_LOCK`]: super::settings_flows::PROVIDER_LOCK +use std::net::SocketAddr; use std::sync::Arc; use axum::body::Body; +use axum::extract::ConnectInfo; use axum::http::{header, HeaderValue, Request, StatusCode}; use axum::Router; use rustasea::auth::users::AuthUserRecord; @@ -131,6 +133,13 @@ fn login_post(email: &str, password: &str) -> Request { request } +/// Attach a client peer address, as the production server does for every request. +fn from_peer(mut request: Request, ip: &str) -> Request { + let addr = SocketAddr::new(ip.parse().expect("peer ip"), 54321); + request.extensions_mut().insert(ConnectInfo(addr)); + request +} + /// Extract the `rustasea-session` cookie value from a response's `Set-Cookie`. fn session_cookie(headers: &axum::http::HeaderMap) -> Option { let raw = headers.get(header::SET_COOKIE)?.to_str().ok()?; @@ -148,7 +157,7 @@ async fn successful_login_records_row() { let logger = Arc::new(AuthenticationLogLogger::new(pool.clone())); let _guard = LoggerGuard::install(logger.clone()); - let mut request = login_post(EMAIL, PASSWORD); + let mut request = from_peer(login_post(EMAIL, PASSWORD), "203.0.113.10"); request .headers_mut() .insert(header::USER_AGENT, HeaderValue::from_static("test-agent")); @@ -160,7 +169,7 @@ async fn successful_login_records_row() { let row = &rows[0]; assert_eq!(row.event, "login_succeeded"); assert_eq!(row.user_id.as_deref(), Some("user-1")); - assert_eq!(row.ip_address.as_deref(), Some("0.0.0.0")); + assert_eq!(row.ip_address.as_deref(), Some("203.0.113.10")); assert_eq!(row.user_agent.as_deref(), Some("test-agent")); assert!(row.successful); assert!(row.login_at.is_some()); @@ -261,3 +270,97 @@ async fn logout_fills_open_row() { ); pool.close().await; } + +/// Send a `POST /login` over a real TCP connection and return the status code. +/// +/// Drives the served socket rather than `Router::oneshot`, so the request goes +/// through the same connection handling as production. +async fn login_over_tcp(addr: SocketAddr, email: &str, password: &str) -> u16 { + use tokio::io::{AsyncReadExt, AsyncWriteExt}; + + let body = format!("email={email}&password={password}"); + let request = format!( + "POST /login HTTP/1.1\r\nhost: {addr}\r\n\ + content-type: application/x-www-form-urlencoded\r\ncontent-length: {}\r\n\ + x-csrf-token: {}\r\nsec-fetch-site: same-origin\r\nconnection: close\r\n\r\n{body}", + body.len(), + crate::routes::helpers::csrf_token(), + ); + let mut stream = tokio::net::TcpStream::connect(addr).await.expect("connect"); + stream + .write_all(request.as_bytes()) + .await + .expect("write request"); + let mut response = Vec::new(); + stream + .read_to_end(&mut response) + .await + .expect("read response"); + String::from_utf8_lossy(&response) + .split_whitespace() + .nth(1) + .and_then(|code| code.parse().ok()) + .expect("status line") +} + +/// The production serve path attaches the connection's peer address, so a +/// login over real TCP is logged with the client IP. Without `ConnectInfo` +/// every request fell back to `0.0.0.0` and shared one throttle bucket. +#[tokio::test] +async fn served_app_logs_the_connection_peer() { + let _lock = super::settings_flows::PROVIDER_LOCK.lock().await; + install_provider(); + let pool = pool_with_table().await; + let logger = Arc::new(AuthenticationLogLogger::new(pool.clone())); + let _guard = LoggerGuard::install(logger.clone()); + + let listener = tokio::net::TcpListener::bind("127.0.0.1:0") + .await + .expect("bind"); + let addr = listener.local_addr().expect("local addr"); + let (stop, stopped) = tokio::sync::oneshot::channel::<()>(); + let server = tokio::spawn(crate::serve(listener, app_with_guard(), async { + let _ = stopped.await; + })); + + let status = login_over_tcp(addr, EMAIL, PASSWORD).await; + let _ = stop.send(()); + server.await.expect("server task").expect("serve"); + + assert_eq!(status, StatusCode::SEE_OTHER.as_u16(), "login must succeed"); + let rows = logger.latest(10).await.expect("query"); + assert_eq!(rows.len(), 1, "exactly one row"); + assert_eq!(rows[0].ip_address.as_deref(), Some("127.0.0.1")); + pool.close().await; +} + +/// The `login` throttle keys on the client peer: five failed attempts lock that +/// client out, while a different client can still log in as the same user. +/// With every request on `0.0.0.0`, any client could lock any username out. +#[tokio::test] +async fn lockout_is_scoped_to_the_client_peer() { + let _lock = super::settings_flows::PROVIDER_LOCK.lock().await; + install_provider(); + + let attacker = "203.0.113.21"; + for _ in 0..5 { + let request = from_peer(login_post(EMAIL, "wrong-password"), attacker); + let (status, _, _) = call(app_with_guard(), request).await; + assert_eq!(status, StatusCode::UNPROCESSABLE_ENTITY); + } + let request = from_peer(login_post(EMAIL, "wrong-password"), attacker); + let (status, _, _) = call(app_with_guard(), request).await; + assert_eq!( + status, + StatusCode::TOO_MANY_REQUESTS, + "the attacking client is throttled" + ); + + let request = from_peer(login_post(EMAIL, PASSWORD), "203.0.113.22"); + let (status, _, _) = call(app_with_guard(), request).await; + assert_eq!( + status, + StatusCode::SEE_OTHER, + "another client still logs in as the same user" + ); +} From 13e8605ea55a8bf9e90715add8091a2eee928ba7 Mon Sep 17 00:00:00 2001 From: kangmaup Date: Thu, 24 Sep 2026 17:09:48 +0700 Subject: [PATCH 12/13] fix(scaffold): serve generated apps with ConnectInfo The generated `main.rs` had the same `axum::serve(listener, router)` call, so scaffolded apps also lost the client IP on every request. It now serves `router.into_make_service_with_connect_info::()`, matching `rustasea-app`; the generated-app compile gate checks every variant. --- crates/rustasea-scaffold/src/templates/core.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/rustasea-scaffold/src/templates/core.rs b/crates/rustasea-scaffold/src/templates/core.rs index 4a2182c..be408b5 100644 --- a/crates/rustasea-scaffold/src/templates/core.rs +++ b/crates/rustasea-scaffold/src/templates/core.rs @@ -459,7 +459,8 @@ async fn main() -> Result<(), Box> { let listener = tokio::net::TcpListener::bind(addr).await?; println!("@@app_pascal@@ listening on http://{addr}"); - axum::serve(listener, router) + // `ConnectInfo` hands every request its client IP (throttle keys, audit logs). + axum::serve(listener, router.into_make_service_with_connect_info::()) .with_graceful_shutdown(app.shutdown()) .await?; Ok(()) From 80d9bee6acf20f6a6ee759b3df30c56e07f9b652 Mon Sep 17 00:00:00 2001 From: kangmaup Date: Thu, 24 Sep 2026 17:09:48 +0700 Subject: [PATCH 13/13] docs: document the CI test job's fetch and no-fail-fast steps The CONTRIBUTING job table and the README still described the `test` job as `cargo test --workspace`; it now runs `cargo fetch` and then `cargo test --workspace --no-fail-fast`. --- CONTRIBUTING.md | 2 +- README.md | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 6e690bf..0b416ec 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -126,7 +126,7 @@ cargo test -p rustasea --features integration -- --ignored | Job | What it runs | |---|---| | `quality` | `cargo xtask ci` (fmt, clippy with `-D warnings`, `deps:check`, `lines:check`, cycle check) | - | `test` | `cargo test --workspace` | + | `test` | `cargo fetch`, then `cargo test --workspace --no-fail-fast` | | `deny` | `cargo deny check` | | `audit` | `cargo audit` | | `msrv` | `cargo check --workspace` on Rust 1.88.0 | diff --git a/README.md b/README.md index 379a41a..5d6e343 100644 --- a/README.md +++ b/README.md @@ -759,7 +759,8 @@ cargo xtask migrate # run migrations ``` CI runs the same gate — see [`.github/workflows/ci.yml`](.github/workflows/ci.yml): -`quality` (`cargo xtask ci`), `test` (`cargo test --workspace`), `deny` +`quality` (`cargo xtask ci`), `test` (`cargo fetch`, then `cargo test --workspace +--no-fail-fast`), `deny` (`cargo deny check`), `audit` (`cargo audit`), and an `msrv` job that checks the workspace builds on the 1.88 floor (ADR-0001). **Formatting violations fail the build**: run `cargo fmt --all` before pushing, or `cargo xtask fmt` to check.