Skip to main content

max / makenotwork

Test passkeys, ssh_keys, and issues db cold spots Extend the db-layer testing pass to the remaining audit cold spots (16 tests): - passkeys: create/find-by-credential roundtrip, post-auth counter bump, and IDOR guards (rename/delete are user-scoped no-ops for a non-owner); make db::passkeys pub for the test crate, matching ssh_keys/issues/synckit - ssh_keys: owner-scoped delete-by-id and delete-by-fingerprint, plus a regression guard for migration 161's global UNIQUE(fingerprint) so one account can't claim another's key fingerprint - issues: repo-scoped sequential numbering, per-repo independence, and two concurrent creates taking distinct numbers via the MAX(number)+1 retry Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Author: Max Johnson <me@maxj.phd> · 2026-07-02 14:41 UTC
Commit: f37e74cbbd11de3e2b9231a8e942af08e3a43f51
Parent: 95370d4
5 files changed, +369 insertions, -1 deletion
M server/src/db/mod.rs +1 -1
@@ -34,7 +34,7 @@ pub(crate) mod tags;
34 34 pub(crate) mod categories;
35 35 pub mod sessions;
36 36 pub(crate) mod totp;
37 - pub(crate) mod passkeys;
37 + pub mod passkeys; // pub so the integration test crate can exercise the layer directly
38 38 pub(crate) mod health;
39 39 pub(crate) mod monitor;
40 40 pub(crate) mod scanning;
A server/tests/workflows/db_issues_layer.rs +114
@@ -0,0 +1,114 @@
1 + //! DB-layer contract tests for `db::issues` — repo-scoped issue numbering.
2 + //!
3 + //! Audit Run 16 flagged `issues` as a Concurrency/Testing cold spot (B+). The
4 + //! interesting invariant is the sequential per-repo `number` assignment:
5 + //! `create_issue` computes `MAX(number)+1` and retries once on the
6 + //! `(repo_id, number)` unique violation that a concurrent insert can cause.
7 + //! These pin monotonic numbering, per-repo independence, and that two
8 + //! simultaneous creates resolve to two distinct numbers (the retry path).
9 + //!
10 + //! NOTE: the retry is single-shot, so it reliably resolves up to two colliding
11 + //! writers; a burst of many simultaneous creates on one repo can still exhaust
12 + //! it. The concurrency test stays within that guaranteed envelope.
13 +
14 + use crate::harness::db::TestDb;
15 + use makenotwork::db::{issues, GitRepoId, UserId};
16 +
17 + async fn seed_user(pool: &sqlx::PgPool, username: &str) -> UserId {
18 + let hash = makenotwork::auth::hash_password("password123").expect("hash");
19 + sqlx::query_scalar::<_, UserId>(
20 + "INSERT INTO users (username, email, password_hash, email_verified)
21 + VALUES ($1, $2, $3, true) RETURNING id",
22 + )
23 + .bind(username)
24 + .bind(format!("{username}@test.com"))
25 + .bind(&hash)
26 + .fetch_one(pool)
27 + .await
28 + .expect("seed user")
29 + }
30 +
31 + async fn seed_repo(pool: &sqlx::PgPool, user: UserId, name: &str) -> GitRepoId {
32 + sqlx::query_scalar::<_, GitRepoId>(
33 + "INSERT INTO git_repos (user_id, name) VALUES ($1, $2) RETURNING id",
34 + )
35 + .bind(user)
36 + .bind(name)
37 + .fetch_one(pool)
38 + .await
39 + .expect("seed repo")
40 + }
41 +
42 + #[tokio::test]
43 + async fn issue_numbers_are_sequential_within_a_repo() {
44 + let db = TestDb::new().await;
45 + let user = seed_user(&db.pool, "iss_seq").await;
46 + let repo = seed_repo(&db.pool, user, "repo").await;
47 +
48 + let mut numbers = Vec::new();
49 + for i in 0..4 {
50 + let issue = issues::create_issue(&db.pool, repo, user, &format!("Issue {i}"), "md", "html")
51 + .await
52 + .unwrap();
53 + numbers.push(issue.number);
54 + }
55 + assert_eq!(numbers, [1, 2, 3, 4], "numbers increment monotonically from 1");
56 + }
57 +
58 + #[tokio::test]
59 + async fn issue_numbering_is_independent_per_repo() {
60 + let db = TestDb::new().await;
61 + let user = seed_user(&db.pool, "iss_scope").await;
62 + let repo_a = seed_repo(&db.pool, user, "repo-a").await;
63 + let repo_b = seed_repo(&db.pool, user, "repo-b").await;
64 +
65 + let a1 = issues::create_issue(&db.pool, repo_a, user, "A1", "m", "h").await.unwrap();
66 + let b1 = issues::create_issue(&db.pool, repo_b, user, "B1", "m", "h").await.unwrap();
67 + let a2 = issues::create_issue(&db.pool, repo_a, user, "A2", "m", "h").await.unwrap();
68 +
69 + // Each repo has its own sequence starting at 1.
70 + assert_eq!(a1.number, 1);
71 + assert_eq!(b1.number, 1, "a second repo's numbering is not affected by the first");
72 + assert_eq!(a2.number, 2);
73 + }
74 +
75 + #[tokio::test]
76 + async fn get_issue_by_number_resolves_within_the_repo() {
77 + let db = TestDb::new().await;
78 + let user = seed_user(&db.pool, "iss_get").await;
79 + let repo = seed_repo(&db.pool, user, "repo").await;
80 + let created = issues::create_issue(&db.pool, repo, user, "Findable", "m", "h").await.unwrap();
81 +
82 + let fetched = issues::get_issue_by_number(&db.pool, repo, created.number)
83 + .await
84 + .unwrap()
85 + .expect("the issue resolves by its repo-scoped number");
86 + assert_eq!(fetched.id, created.id);
87 + assert_eq!(fetched.title, "Findable");
88 +
89 + // A number that doesn't exist in this repo resolves to None.
90 + assert!(issues::get_issue_by_number(&db.pool, repo, 999).await.unwrap().is_none());
91 + }
92 +
93 + /// Two simultaneous creates on the same repo must not collide on a number: the
94 + /// `MAX(number)+1` race is resolved by the single-shot retry, so both succeed
95 + /// with distinct sequential numbers {1, 2}.
96 + #[tokio::test]
97 + async fn concurrent_creates_get_distinct_numbers() {
98 + let db = TestDb::new().await;
99 + let user = seed_user(&db.pool, "iss_race").await;
100 + let repo = seed_repo(&db.pool, user, "repo").await;
101 +
102 + let p1 = db.pool.clone();
103 + let p2 = db.pool.clone();
104 + let (a, b) = tokio::join!(
105 + tokio::spawn(async move { issues::create_issue(&p1, repo, user, "A", "m", "h").await }),
106 + tokio::spawn(async move { issues::create_issue(&p2, repo, user, "B", "m", "h").await }),
107 + );
108 + let a = a.unwrap().expect("first create must succeed");
109 + let b = b.unwrap().expect("second create must succeed under contention");
110 +
111 + let mut nums = [a.number, b.number];
112 + nums.sort();
113 + assert_eq!(nums, [1, 2], "concurrent creates take two distinct sequential numbers");
114 + }
A server/tests/workflows/db_passkeys_layer.rs +131
@@ -0,0 +1,131 @@
1 + //! DB-layer contract tests for `db::passkeys` — WebAuthn credential storage.
2 + //!
3 + //! Audit Run 16 graded `db/passkeys.rs` A- with "thin module tests": the
4 + //! ownership-scoped mutations (rename/delete return false for a non-owner) and
5 + //! the discoverable-login lookup were exercised only through the HTTP passkey
6 + //! flow. These pin the invariants directly — most importantly that rename and
7 + //! delete are user-scoped (an IDOR guard), the credential-id lookup resolves the
8 + //! owner, and the post-auth update bumps the stored counter.
9 +
10 + use crate::harness::db::TestDb;
11 + use makenotwork::db::{passkeys, UserId};
12 + use serde_json::json;
13 +
14 + async fn seed_user(pool: &sqlx::PgPool, username: &str) -> UserId {
15 + let hash = makenotwork::auth::hash_password("password123").expect("hash");
16 + sqlx::query_scalar::<_, UserId>(
17 + "INSERT INTO users (username, email, password_hash, email_verified)
18 + VALUES ($1, $2, $3, true) RETURNING id",
19 + )
20 + .bind(username)
21 + .bind(format!("{username}@test.com"))
22 + .bind(&hash)
23 + .fetch_one(pool)
24 + .await
25 + .expect("seed user")
26 + }
27 +
28 + #[tokio::test]
29 + async fn create_then_find_by_credential_id_roundtrips_the_owner() {
30 + let db = TestDb::new().await;
31 + let user = seed_user(&db.pool, "pk_roundtrip").await;
32 + let cred = b"cred-roundtrip".as_slice();
33 +
34 + passkeys::create_passkey(&db.pool, user, "Yubikey", &json!({"counter": 0}), cred)
35 + .await
36 + .unwrap();
37 +
38 + let found = passkeys::find_user_by_credential_id(&db.pool, cred).await.unwrap();
39 + let (owner, stored) = found.expect("a stored credential resolves to its owner");
40 + assert_eq!(owner, user);
41 + assert_eq!(stored, json!({"counter": 0}));
42 +
43 + // An unknown credential id resolves to nobody (no discoverable-login match).
44 + assert!(
45 + passkeys::find_user_by_credential_id(&db.pool, b"nope".as_slice())
46 + .await
47 + .unwrap()
48 + .is_none()
49 + );
50 + }
51 +
52 + #[tokio::test]
53 + async fn update_after_auth_bumps_the_counter_and_stamps_last_used() {
54 + let db = TestDb::new().await;
55 + let user = seed_user(&db.pool, "pk_counter").await;
56 + let cred = b"cred-counter".as_slice();
57 + passkeys::create_passkey(&db.pool, user, "Key", &json!({"counter": 4}), cred).await.unwrap();
58 +
59 + // Pre-auth: last_used_at is NULL.
60 + let before = passkeys::list_passkeys(&db.pool, user).await.unwrap();
61 + assert_eq!(before.len(), 1);
62 + assert!(before[0].last_used_at.is_none());
63 +
64 + passkeys::update_passkey_after_auth(&db.pool, cred, &json!({"counter": 5})).await.unwrap();
65 +
66 + let (_, stored) = passkeys::find_user_by_credential_id(&db.pool, cred).await.unwrap().unwrap();
67 + assert_eq!(stored, json!({"counter": 5}), "the signature counter is advanced");
68 + let after = passkeys::list_passkeys(&db.pool, user).await.unwrap();
69 + assert!(after[0].last_used_at.is_some(), "last_used_at is stamped on auth");
70 + }
71 +
72 + /// The IDOR guard: one user must not be able to delete another user's passkey
73 + /// by id. `delete_passkey` scopes on `(id, user_id)`, so a non-owner's delete is
74 + /// a no-op that returns false — the credential survives.
75 + #[tokio::test]
76 + async fn delete_is_scoped_to_the_owner() {
77 + let db = TestDb::new().await;
78 + let owner = seed_user(&db.pool, "pk_owner").await;
79 + let attacker = seed_user(&db.pool, "pk_attacker").await;
80 + let pk = passkeys::create_passkey(&db.pool, owner, "Key", &json!({}), b"c-own".as_slice())
81 + .await
82 + .unwrap();
83 +
84 + // Attacker tries to delete the owner's passkey by id -> no-op.
85 + assert!(
86 + !passkeys::delete_passkey(&db.pool, pk, attacker).await.unwrap(),
87 + "a non-owner delete must return false"
88 + );
89 + assert_eq!(passkeys::count_passkeys(&db.pool, owner).await.unwrap(), 1, "the credential survives");
90 +
91 + // The owner can delete it.
92 + assert!(passkeys::delete_passkey(&db.pool, pk, owner).await.unwrap());
93 + assert_eq!(passkeys::count_passkeys(&db.pool, owner).await.unwrap(), 0);
94 + }
95 +
96 + #[tokio::test]
97 + async fn rename_is_scoped_to_the_owner() {
98 + let db = TestDb::new().await;
99 + let owner = seed_user(&db.pool, "pk_rn_owner").await;
100 + let attacker = seed_user(&db.pool, "pk_rn_attacker").await;
101 + let pk = passkeys::create_passkey(&db.pool, owner, "Original", &json!({}), b"c-rn".as_slice())
102 + .await
103 + .unwrap();
104 +
105 + assert!(
106 + !passkeys::rename_passkey(&db.pool, pk, attacker, "Pwned").await.unwrap(),
107 + "a non-owner rename must return false"
108 + );
109 + let names: Vec<_> = passkeys::list_passkeys(&db.pool, owner).await.unwrap();
110 + assert_eq!(names[0].name, "Original", "the name is unchanged by a non-owner");
111 +
112 + assert!(passkeys::rename_passkey(&db.pool, pk, owner, "Renamed").await.unwrap());
113 + let names: Vec<_> = passkeys::list_passkeys(&db.pool, owner).await.unwrap();
114 + assert_eq!(names[0].name, "Renamed");
115 + }
116 +
117 + #[tokio::test]
118 + async fn credential_exclusion_list_is_per_user() {
119 + let db = TestDb::new().await;
120 + let a = seed_user(&db.pool, "pk_excl_a").await;
121 + let b = seed_user(&db.pool, "pk_excl_b").await;
122 + passkeys::create_passkey(&db.pool, a, "A1", &json!({"id": "a1"}), b"a1".as_slice()).await.unwrap();
123 + passkeys::create_passkey(&db.pool, a, "A2", &json!({"id": "a2"}), b"a2".as_slice()).await.unwrap();
124 + passkeys::create_passkey(&db.pool, b, "B1", &json!({"id": "b1"}), b"b1".as_slice()).await.unwrap();
125 +
126 + // The registration exclusion list must contain only the caller's credentials.
127 + let a_creds = passkeys::get_passkey_credentials(&db.pool, a).await.unwrap();
128 + assert_eq!(a_creds.len(), 2);
129 + assert!(a_creds.iter().all(|c| c["id"].as_str().unwrap().starts_with('a')));
130 + assert_eq!(passkeys::count_passkeys(&db.pool, b).await.unwrap(), 1);
131 + }
A server/tests/workflows/db_ssh_keys_layer.rs +120
@@ -0,0 +1,120 @@
1 + //! DB-layer contract tests for `db::ssh_keys` — git-over-SSH key CRUD + lookup.
2 + //!
3 + //! Audit Run 16 flagged `db/ssh_keys.rs:98` (`lookup_user_by_fingerprint`) and
4 + //! the module's thin coverage. These pin the ownership-scoped deletes (an IDOR
5 + //! guard on the git surface), the per-user duplicate rejection, and the global
6 + //! fingerprint uniqueness (migration 161) that keeps `lookup_user_by_fingerprint`
7 + //! unambiguous — one account can't register another's key fingerprint.
8 +
9 + use crate::harness::db::TestDb;
10 + use makenotwork::db::{ssh_keys, UserId};
11 +
12 + async fn seed_user(pool: &sqlx::PgPool, username: &str) -> UserId {
13 + let hash = makenotwork::auth::hash_password("password123").expect("hash");
14 + sqlx::query_scalar::<_, UserId>(
15 + "INSERT INTO users (username, email, password_hash, email_verified)
16 + VALUES ($1, $2, $3, true) RETURNING id",
17 + )
18 + .bind(username)
19 + .bind(format!("{username}@test.com"))
20 + .bind(&hash)
21 + .fetch_one(pool)
22 + .await
23 + .expect("seed user")
24 + }
25 +
26 + #[tokio::test]
27 + async fn add_then_lookup_resolves_the_owner() {
28 + let db = TestDb::new().await;
29 + let user = seed_user(&db.pool, "ssh_lookup").await;
30 +
31 + ssh_keys::add_key(&db.pool, user, "ssh-ed25519 AAAA", "SHA256:aaa", "laptop")
32 + .await
33 + .unwrap();
34 +
35 + let found = ssh_keys::lookup_user_by_fingerprint(&db.pool, "SHA256:aaa").await.unwrap();
36 + assert_eq!(found.expect("known fingerprint resolves").user_id, user);
37 +
38 + // An unknown fingerprint authenticates nobody.
39 + assert!(
40 + ssh_keys::lookup_user_by_fingerprint(&db.pool, "SHA256:unknown")
41 + .await
42 + .unwrap()
43 + .is_none()
44 + );
45 + }
46 +
47 + #[tokio::test]
48 + async fn duplicate_fingerprint_for_same_user_is_rejected() {
49 + let db = TestDb::new().await;
50 + let user = seed_user(&db.pool, "ssh_dup").await;
51 +
52 + ssh_keys::add_key(&db.pool, user, "ssh-ed25519 AAAA", "SHA256:dup", "one").await.unwrap();
53 + // UNIQUE (user_id, fingerprint) — re-registering the same key fails.
54 + let dup = ssh_keys::add_key(&db.pool, user, "ssh-ed25519 AAAA", "SHA256:dup", "two").await;
55 + assert!(dup.is_err(), "the same fingerprint cannot be registered twice by one user");
56 + }
57 +
58 + /// A fingerprint maps to exactly ONE identity, globally. Migration 161 added a
59 + /// global `UNIQUE (fingerprint)` on top of the original `(user_id, fingerprint)`
60 + /// specifically so one account can't register a public key already tied to
61 + /// another account and thereby authenticate as it (Run 15 Auth C1). This guards
62 + /// that fix: a second user registering the same fingerprint is rejected, so
63 + /// `lookup_user_by_fingerprint` can never be ambiguous.
64 + #[tokio::test]
65 + async fn fingerprint_is_globally_unique_across_users() {
66 + let db = TestDb::new().await;
67 + let first = seed_user(&db.pool, "ssh_first").await;
68 + let second = seed_user(&db.pool, "ssh_second").await;
69 +
70 + ssh_keys::add_key(&db.pool, first, "ssh-ed25519 AAAA", "SHA256:shared", "first").await.unwrap();
71 + // A different account cannot claim the same fingerprint.
72 + let stolen = ssh_keys::add_key(&db.pool, second, "ssh-ed25519 AAAA", "SHA256:shared", "second").await;
73 + assert!(
74 + stolen.is_err(),
75 + "a fingerprint already registered to one account must not be registrable by another"
76 + );
77 +
78 + // The lookup remains unambiguous — it resolves to the sole registrant.
79 + let resolved = ssh_keys::lookup_user_by_fingerprint(&db.pool, "SHA256:shared")
80 + .await
81 + .unwrap()
82 + .expect("fingerprint resolves");
83 + assert_eq!(resolved.user_id, first);
84 + }
85 +
86 + /// IDOR guard: `delete_key` scopes on `(id, user_id)`, so one user cannot delete
87 + /// another's key by id.
88 + #[tokio::test]
89 + async fn delete_by_id_is_scoped_to_the_owner() {
90 + let db = TestDb::new().await;
91 + let owner = seed_user(&db.pool, "ssh_owner").await;
92 + let attacker = seed_user(&db.pool, "ssh_attacker").await;
93 + let key = ssh_keys::add_key(&db.pool, owner, "ssh-ed25519 AAAA", "SHA256:own", "k").await.unwrap();
94 +
95 + assert!(
96 + !ssh_keys::delete_key(&db.pool, key.id, attacker).await.unwrap(),
97 + "a non-owner delete-by-id must return false"
98 + );
99 + assert_eq!(ssh_keys::list_keys_by_user(&db.pool, owner).await.unwrap().len(), 1, "the key survives");
100 +
101 + assert!(ssh_keys::delete_key(&db.pool, key.id, owner).await.unwrap());
102 + assert!(ssh_keys::list_keys_by_user(&db.pool, owner).await.unwrap().is_empty());
103 + }
104 +
105 + #[tokio::test]
106 + async fn delete_by_fingerprint_is_scoped_to_the_owner() {
107 + let db = TestDb::new().await;
108 + let owner = seed_user(&db.pool, "ssh_fp_owner").await;
109 + let attacker = seed_user(&db.pool, "ssh_fp_attacker").await;
110 + ssh_keys::add_key(&db.pool, owner, "ssh-ed25519 AAAA", "SHA256:fp", "k").await.unwrap();
111 +
112 + // The attacker knows the fingerprint but not the owner scope: no-op.
113 + assert!(
114 + !ssh_keys::delete_key_by_fingerprint(&db.pool, attacker, "SHA256:fp").await.unwrap(),
115 + "a non-owner delete-by-fingerprint must return false"
116 + );
117 + assert_eq!(ssh_keys::list_keys_by_user(&db.pool, owner).await.unwrap().len(), 1);
118 +
119 + assert!(ssh_keys::delete_key_by_fingerprint(&db.pool, owner, "SHA256:fp").await.unwrap());
120 + }
M server/tests/workflows/mod.rs +3
@@ -99,6 +99,9 @@ mod bundles;
99 99 mod idempotency;
100 100 mod db_payments_layer;
101 101 mod db_creator_tiers_layer;
102 + mod db_issues_layer;
103 + mod db_passkeys_layer;
104 + mod db_ssh_keys_layer;
102 105 mod db_synckit_rotation;
103 106 mod db_webhook_events;
104 107 mod security_idor_moderation;