Skip to main content

max / makenotwork

audit Run 14 Phase 4: git push-gate unit tests + concurrent license-activation races - git: extract require_push_token (pure token-scope gate) from authorize_push and unit-test the no-DB half: anonymous -> 401, session cookie (None) -> 403, read-only token -> 403, push-scoped token -> Ok. Collaborator DB branch unchanged. - license_keys: two concurrency integration tests on try_create_activation (FOR UPDATE + full COUNT): 16 distinct machines racing a single-seat key yield exactly one winner + count==1; 8 concurrent same-machine re-activations stay idempotent at one seat. Uses existing cached DB fns (no new query! macro). Verified against Postgres. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Author: Max Johnson <me@maxj.phd> · 2026-07-01 23:00 UTC
Commit: fca9902cbeeba0409cc285515afdfdcd97d90cf1
Parent: 9038c15
2 files changed, +131 insertions, -6 deletions
M server/src/routes/git/raw.rs +55 -6
@@ -159,6 +159,22 @@ pub(super) async fn smart_http_info_refs(
159 159 .context("build git http response")
160 160 }
161 161
162 + /// Pure token-scope gate for smart-HTTP push: only a push-scoped personal access
163 + /// token may push. Session-cookie auth (`token_push == None`) and read-only
164 + /// tokens (`Some(false)`) are rejected here; the owner/collaborator check runs
165 + /// after, against the token's `user_id`. Split out from [`authorize_push`] so the
166 + /// no-DB authentication half is unit-testable independent of the collaborator DB
167 + /// lookup.
168 + fn require_push_token(
169 + principal: Option<&super::GitHttpPrincipal>,
170 + ) -> Result<&super::GitHttpPrincipal> {
171 + let principal = principal.ok_or(AppError::Unauthorized)?;
172 + if principal.token_push != Some(true) {
173 + return Err(AppError::Forbidden);
174 + }
175 + Ok(principal)
176 + }
177 +
162 178 /// Authorize a push (receive-pack) for the resolved principal. Requires a
163 179 /// personal access token carrying push scope (`token_push == Some(true)`), and
164 180 /// the user must be the repo owner or a push-collaborator — the same write model
@@ -176,12 +192,7 @@ async fn authorize_push(
176 192 resolved: &super::ResolvedRepo,
177 193 principal: Option<&super::GitHttpPrincipal>,
178 194 ) -> Result<()> {
179 - let principal = principal.ok_or(AppError::Unauthorized)?;
180 - if principal.token_push != Some(true) {
181 - // Reject session-cookie auth (None) and read-only tokens (Some(false)):
182 - // only a push-scoped PAT may push over smart-HTTP.
183 - return Err(AppError::Forbidden);
184 - }
195 + let principal = require_push_token(principal)?;
185 196 let is_owner = resolved.db_user.id == principal.user_id;
186 197 if !is_owner {
187 198 let can_push = crate::db::repo_collaborators::can_user_push(
@@ -363,3 +374,41 @@ pub(super) async fn smart_http_receive_pack(
363 374 .body(Body::from_stream(ReaderStream::new(stdout)))
364 375 .context("build git http response")
365 376 }
377 +
378 + #[cfg(test)]
379 + mod tests {
380 + use super::*;
381 + use crate::db::UserId;
382 + use crate::routes::git::GitHttpPrincipal;
383 +
384 + fn principal(token_push: Option<bool>) -> GitHttpPrincipal {
385 + GitHttpPrincipal { user_id: UserId::default(), token_push }
386 + }
387 +
388 + #[test]
389 + fn push_gate_rejects_anonymous() {
390 + // No principal at all (anonymous / unresolved credentials) -> 401.
391 + assert!(matches!(require_push_token(None), Err(AppError::Unauthorized)));
392 + }
393 +
394 + #[test]
395 + fn push_gate_rejects_session_cookie() {
396 + // Session-cookie auth carries token_push == None: never a push credential
397 + // over smart-HTTP (this is the CSRF backstop for cross-origin git POSTs).
398 + let p = principal(None);
399 + assert!(matches!(require_push_token(Some(&p)), Err(AppError::Forbidden)));
400 + }
401 +
402 + #[test]
403 + fn push_gate_rejects_readonly_token() {
404 + let p = principal(Some(false));
405 + assert!(matches!(require_push_token(Some(&p)), Err(AppError::Forbidden)));
406 + }
407 +
408 + #[test]
409 + fn push_gate_accepts_push_scoped_token() {
410 + let p = principal(Some(true));
411 + let got = require_push_token(Some(&p)).expect("push-scoped token passes the gate");
412 + assert_eq!(got.user_id, p.user_id);
413 + }
414 + }
M server/tests/workflows/license_keys.rs +76
@@ -393,3 +393,79 @@ async fn v1_license_endpoints() {
393 393 let v: Value = resp.json();
394 394 assert_eq!(v["success"], true, "v1 deactivate should return success=true");
395 395 }
396 +
397 + /// Concurrent activation race: for a single-seat key, many machines activating
398 + /// at once must resolve to exactly one winner. `try_create_activation` takes the
399 + /// `license_keys` row `FOR UPDATE` and recomputes the count under the lock, so
400 + /// the cap holds even when every request reads the pre-lock count of 0.
401 + #[tokio::test]
402 + async fn concurrent_activation_race_respects_single_seat() {
403 + use makenotwork::db::{self, KeyCode};
404 +
405 + let mut h = TestHarness::new().await;
406 + let item_id = setup_creator_with_item(&mut h, "raceact", 1).await;
407 + let key = generate_key(&mut h, &item_id).await;
408 + let key_code = KeyCode::new(key["key_code"].as_str().unwrap()).unwrap();
409 +
410 + let key_row = db::license_keys::get_license_key_by_code(&h.db, &key_code)
411 + .await
412 + .unwrap()
413 + .expect("generated key exists");
414 + let key_id = key_row.id;
415 +
416 + // Fire many concurrent activations, each on a distinct machine.
417 + let mut handles = Vec::new();
418 + for i in 0..16 {
419 + let pool = h.db.clone();
420 + handles.push(tokio::spawn(async move {
421 + db::license_keys::try_create_activation(&pool, key_id, &format!("machine-{i}"), Some("race"))
422 + .await
423 + .unwrap()
424 + }));
425 + }
426 +
427 + let mut granted = 0;
428 + for handle in handles {
429 + if handle.await.unwrap().is_some() {
430 + granted += 1;
431 + }
432 + }
433 + assert_eq!(granted, 1, "exactly one machine wins the single seat under FOR UPDATE");
434 +
435 + let count = db::license_keys::get_activation_count(&h.db, key_id).await.unwrap();
436 + assert_eq!(count, 1, "denormalized activation_count matches the single winner");
437 + }
438 +
439 + /// Re-activating the same machine is idempotent and never consumes extra seats,
440 + /// even racing against itself — the machine_id upsert path returns the existing
441 + /// row rather than allocating a new activation.
442 + #[tokio::test]
443 + async fn concurrent_same_machine_reactivation_is_idempotent() {
444 + use makenotwork::db::{self, KeyCode};
445 +
446 + let mut h = TestHarness::new().await;
447 + let item_id = setup_creator_with_item(&mut h, "react", 1).await;
448 + let key = generate_key(&mut h, &item_id).await;
449 + let key_code = KeyCode::new(key["key_code"].as_str().unwrap()).unwrap();
450 + let key_id = db::license_keys::get_license_key_by_code(&h.db, &key_code)
451 + .await
452 + .unwrap()
453 + .unwrap()
454 + .id;
455 +
456 + let mut handles = Vec::new();
457 + for _ in 0..8 {
458 + let pool = h.db.clone();
459 + handles.push(tokio::spawn(async move {
460 + db::license_keys::try_create_activation(&pool, key_id, "same-machine", Some("dup"))
461 + .await
462 + .unwrap()
463 + }));
464 + }
465 + for handle in handles {
466 + assert!(handle.await.unwrap().is_some(), "same-machine re-activation always succeeds");
467 + }
468 +
469 + let count = db::license_keys::get_activation_count(&h.db, key_id).await.unwrap();
470 + assert_eq!(count, 1, "one machine == one seat regardless of concurrent re-activations");
471 + }