Merge pull request #298 from euxaristia/fix/get-profile-strict-did-matching

fix: strict DID matching in get_profile

Author: kevincodex1

Cargo.lock

@@ -917,6 +917,10 @@ pub(crate) fn normalize_owner_key(did: &str) -> &str {
 /// drift apart. If you change `normalize_owner_key`, update this const too.
 const OWNER_KEY_CASE_SQL: &str = "CASE WHEN owner_did LIKE 'did:key:%' AND position(':' in substr(owner_did, 9)) = 0 THEN substr(owner_did, 9) ELSE owner_did END";
 
+/// SQL CASE expression byte-identical to `normalize_owner_key`, but for columns
+/// named `did` (like in agent_profiles) instead of `owner_did`.
+const PROFILE_DID_CASE_SQL: &str = "CASE WHEN did LIKE 'did:key:%' AND position(':' in substr(did, 9)) = 0 THEN substr(did, 9) ELSE did END";
+
 #[cfg(test)]
 mod normalize_owner_key_tests {
     use super::normalize_owner_key;
@@ -3454,6 +3458,9 @@ impl Db {
             let new_website = website.or(existing.website.as_deref());
             let new_socials = socials.or(existing.socials.as_deref());
 
+            // get_profile equates bare short ids with did:key:<id>. UPDATE must target
+            // the stored row identity (existing.did), not the caller's raw input form,
+            // or a did:key: alias against a bare-stored profile updates zero rows.
             sqlx::query(
                 "UPDATE agent_profiles
                  SET display_name=$1, bio=$2, avatar_url=$3, website=$4, socials=$5, updated_at=$6
@@ -3465,12 +3472,12 @@ impl Db {
             .bind(new_website)
             .bind(new_socials)
             .bind(&now)
-            .bind(did)
+            .bind(&existing.did)
             .execute(&self.pool)
             .await?;
 
             Ok(ProfileRecord {
-                did: did.to_string(),
+                did: existing.did,
                 display_name: new_name.map(String::from),
                 bio: new_bio.map(String::from),
                 avatar_url: new_avatar.map(String::from),
@@ -3511,14 +3518,20 @@ impl Db {
     }
 
     pub async fn get_profile(&self, did: &str) -> Result<Option<ProfileRecord>> {
-        let row = sqlx::query(
+        // Same owner-key contract as get_repo: strip `did:key:` only when the
+        // remainder is a bare key id. The old `LIKE '%:' || $1` matched any DID
+        // method that shared a suffix and could resolve the wrong profile.
+        let did_key = normalize_owner_key(did);
+        let sql = format!(
             "SELECT did, display_name, bio, avatar_url, website, socials, profile_cid, created_at, updated_at
              FROM agent_profiles
-             WHERE did = $1 OR did LIKE '%:' || $1",
-        )
-        .bind(did)
-        .fetch_optional(&self.pool)
-        .await?;
+             WHERE ({key}) = $1",
+            key = PROFILE_DID_CASE_SQL
+        );
+        let row = sqlx::query(&sql)
+            .bind(did_key)
+            .fetch_optional(&self.pool)
+            .await?;
 
         Ok(row.map(|r| ProfileRecord {
             did: r.get("did"),
@@ -3534,10 +3547,17 @@ impl Db {
     }
 
     pub async fn set_profile_cid(&self, did: &str, cid: &str) -> Result<()> {
-        sqlx::query("UPDATE agent_profiles SET profile_cid = $1, updated_at = $2 WHERE did = $3")
+        // Same did:key / bare equivalence as get_profile so a full did:key: form
+        // updates a profile stored under the bare short id.
+        let did_key = normalize_owner_key(did);
+        let sql = format!(
+            "UPDATE agent_profiles SET profile_cid = $1, updated_at = $2 WHERE ({key}) = $3",
+            key = PROFILE_DID_CASE_SQL
+        );
+        sqlx::query(&sql)
             .bind(cid)
             .bind(Utc::now().to_rfc3339())
-            .bind(did)
+            .bind(did_key)
             .execute(&self.pool)
             .await?;
         Ok(())
@@ -4671,6 +4691,103 @@ mod dedup_db_tests {
         assert!(!got.is_public, "non-key row's is_public must be preserved");
     }
 
+    /// get_profile must not resolve a non-key DID (e.g. did:gitlawb:) when
+    /// queried with the bare short id. The old `LIKE '%:' || $1` clause was too
+    /// broad and could return the wrong profile row.
+    #[sqlx::test]
+    async fn get_profile_does_not_match_non_key_did(pool: PgPool) {
+        let db = db(pool).await;
+        let short = "z6Mkprof1";
+
+        // Seed only a non-key DID row first. When queried with the bare short ID,
+        // get_profile must return None (lone non-key fixture test).
+        db.upsert_profile(
+            &format!("did:gitlawb:{short}"),
+            Some("other-method"),
+            None,
+            None,
+            None,
+            None,
+        )
+        .await
+        .unwrap();
+
+        let got = db.get_profile(short).await.unwrap();
+        assert!(
+            got.is_none(),
+            "bare short id must not resolve a lone non-key DID profile"
+        );
+
+        // Now seed the canonical bare key profile row as well.
+        db.upsert_profile(short, Some("canonical"), None, None, None, None)
+            .await
+            .unwrap();
+
+        let got = db
+            .get_profile(short)
+            .await
+            .unwrap()
+            .expect("bare short id should resolve the key-form profile");
+        assert_eq!(got.did, short);
+        assert_eq!(got.display_name.as_deref(), Some("canonical"));
+
+        let got = db
+            .get_profile(&format!("did:key:{short}"))
+            .await
+            .unwrap()
+            .expect("did:key form should also resolve the key-form profile");
+        assert_eq!(got.did, short);
+
+        let got = db
+            .get_profile(&format!("did:gitlawb:{short}"))
+            .await
+            .unwrap()
+            .expect("full non-key DID should resolve its own profile");
+        assert_eq!(got.did, format!("did:gitlawb:{short}"));
+        assert_eq!(got.display_name.as_deref(), Some("other-method"));
+    }
+
+    /// upsert_profile must update a bare-stored profile when called with the
+    /// full did:key: form. get_profile equates the two, so the UPDATE has to
+    /// target existing.did rather than the raw input.
+    #[sqlx::test]
+    async fn upsert_profile_updates_via_did_key_alias(pool: PgPool) {
+        let db = db(pool).await;
+        let short = "z6Mkprof2";
+
+        db.upsert_profile(short, Some("before"), None, None, None, None)
+            .await
+            .unwrap();
+
+        let updated = db
+            .upsert_profile(
+                &format!("did:key:{short}"),
+                Some("after"),
+                None,
+                None,
+                None,
+                None,
+            )
+            .await
+            .unwrap();
+        assert_eq!(updated.did, short, "preserve the stored did form on update");
+        assert_eq!(updated.display_name.as_deref(), Some("after"));
+
+        let got = db
+            .get_profile(short)
+            .await
+            .unwrap()
+            .expect("profile should still resolve by bare short id");
+        assert_eq!(got.did, short);
+        assert_eq!(got.display_name.as_deref(), Some("after"));
+
+        db.set_profile_cid(&format!("did:key:{short}"), "bafytestcid")
+            .await
+            .unwrap();
+        let got = db.get_profile(short).await.unwrap().unwrap();
+        assert_eq!(got.profile_cid.as_deref(), Some("bafytestcid"));
+    }
+
     /// Verify that the Rust `normalize_owner_key` and the `OWNER_KEY_CASE_SQL`
     /// expression agree on every boundary value in the owner-key normalization
     /// set. A mismatch would let the Rust code bind a different key than the SQL
@@ -4720,6 +4837,49 @@ mod dedup_db_tests {
             );
         }
     }
+
+    /// Verify that `PROFILE_DID_CASE_SQL` (which aliases the column `did`) also
+    /// agrees with Rust `normalize_owner_key` across the full boundary matrix.
+    #[sqlx::test]
+    async fn profile_did_case_sql_matches_normalize_owner_key(pool: PgPool) {
+        let boundary_values = [
+            "did:key:z6Mkfoo",
+            "z6Mkfoo",
+            "did:gitlawb:z6Mkfoo",
+            "did:web:example.com:alice",
+            "did:key:did:gitlawb:z6Mkfoo",
+            "",
+            "did:key:",
+            "DID:KEY:z6Mkfoo",
+        ];
+
+        let values_sql: String = boundary_values
+            .iter()
+            .map(|v| format!("('{}'::text)", v))
+            .collect::<Vec<_>>()
+            .join(", ");
+        let sql = format!(
+            "WITH data(did) AS (VALUES {values_sql})
+             SELECT did, ({key}) AS normalized FROM data ORDER BY did",
+            key = super::PROFILE_DID_CASE_SQL
+        );
+
+        let rows: Vec<(String, String)> = sqlx::query_as(&sql).fetch_all(&pool).await.unwrap();
+
+        assert_eq!(
+            rows.len(),
+            boundary_values.len(),
+            "every boundary value must produce a row"
+        );
+
+        for (val, sql_result) in &rows {
+            let rust_result = super::normalize_owner_key(val);
+            assert_eq!(
+                sql_result, rust_result,
+                "PROFILE_DID_CASE_SQL(\"{val}\") mismatch: Rust = \"{rust_result}\", SQL CASE = \"{sql_result}\""
+            );
+        }
+    }
 }
 
 /// Exercises the iCaptcha single-use proof ledger (`icaptcha_consumed_proofs`),