Skip to content

Commit 9c3df2c

Browse files
committed
fix(node): prefer canonical repo row over mirror row in get_repo (#124)
get_repo matched both mirror rows (bare short-DID, is_public=true, no visibility rules) and canonical rows (full did:key: DID, correct rules), returning one nondeterministically. When the mirror row won, the visibility gate saw is_public=true + empty rules, allowing unauthenticated read of private/withheld content. Add ORDER BY to prefer canonical rows (UUID id, no slash) over mirror rows (slash-form id), matching the ranking logic already used by DEDUP_CTE and dedupe_canonical_repos. Test seeds mirror FIRST so an unordered fetch_optional returns the mirror row — proving the test locks in the fix.
1 parent cd4680e commit 9c3df2c

2 files changed

Lines changed: 84 additions & 6 deletions

File tree

‎Cargo.lock‎

Lines changed: 5 additions & 5 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎crates/gitlawb-node/src/db/mod.rs‎

Lines changed: 79 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -891,7 +891,13 @@ impl Db {
891891
"SELECT id, name, owner_did, description, is_public, default_branch,
892892
created_at, updated_at, disk_path, forked_from, machine_id
893893
FROM repos
894-
WHERE (owner_did = $1 OR owner_did LIKE '%:' || $1 || '%') AND name = $2",
894+
WHERE (owner_did = $1 OR owner_did LIKE '%:' || $1 || '%') AND name = $2
895+
-- Prefer canonical rows (UUID id, no slash) over mirror rows (slash id).
896+
-- Mirror rows are inserted by upsert_mirror_repo with is_public=true and
897+
-- no visibility rules; using them for visibility checks would bypass the
898+
-- canonical row's gate (issue #124).
899+
ORDER BY CASE WHEN position('/' in id) > 0 THEN 1 ELSE 0 END,
900+
created_at ASC, id ASC",
895901
)
896902
.bind(owner_did)
897903
.bind(name)
@@ -3700,6 +3706,78 @@ mod dedup_db_tests {
37003706
assert_eq!(list_len, 2);
37013707
assert_eq!(count, list_len, "count must equal the deduped list length");
37023708
}
3709+
3710+
/// get_repo must prefer the canonical row over the mirror row when both match,
3711+
/// so the visibility gate keys off the canonical row's rules and is_public flag
3712+
/// rather than the mirror's hardcoded public-with-no-rules (issue #124).
3713+
#[sqlx::test]
3714+
async fn get_repo_prefers_canonical_over_mirror(pool: PgPool) {
3715+
let db = db(pool).await;
3716+
let short = "z6Mkwbud";
3717+
let owner_did = "did:key:z6Mkwbud";
3718+
3719+
// Mirror row seeded FIRST — hardcoded is_public=true, no visibility rules.
3720+
// Without the ORDER BY fix, fetch_optional returns this row by insertion order,
3721+
// so the test fails (proving it locks in the fix).
3722+
db.upsert_mirror_repo(short, "secret-repo", "/srv/mirror", None, false)
3723+
.await
3724+
.unwrap();
3725+
3726+
// Canonical row with is_public=false.
3727+
let canonical = RepoRecord {
3728+
id: uuid::Uuid::new_v4().to_string(),
3729+
name: "secret-repo".into(),
3730+
owner_did: owner_did.to_string(),
3731+
description: None,
3732+
is_public: false,
3733+
default_branch: "main".into(),
3734+
created_at: ts("2026-01-01T00:00:00Z"),
3735+
updated_at: ts("2026-01-01T00:00:00Z"),
3736+
disk_path: "/srv/secret".into(),
3737+
forked_from: None,
3738+
machine_id: None,
3739+
};
3740+
db.create_repo(&canonical).await.unwrap();
3741+
3742+
// Querying with bare short DID should return the canonical row.
3743+
let got = db
3744+
.get_repo(short, "secret-repo")
3745+
.await
3746+
.unwrap()
3747+
.expect("get_repo should find the repo");
3748+
3749+
assert_eq!(
3750+
got.owner_did, owner_did,
3751+
"canonical row (did:key: form) must win over mirror row (bare short DID)"
3752+
);
3753+
assert!(
3754+
!got.id.contains('/'),
3755+
"canonical row id must not contain a slash"
3756+
);
3757+
assert!(
3758+
!got.is_public,
3759+
"canonical row's is_public must be preserved"
3760+
);
3761+
}
3762+
3763+
/// get_repo still returns the mirror row when no canonical row exists
3764+
/// (mirror-only group), so sync and read paths remain functional.
3765+
#[sqlx::test]
3766+
async fn get_repo_returns_mirror_when_no_canonical(pool: PgPool) {
3767+
let db = db(pool).await;
3768+
db.upsert_mirror_repo("z6Lonely", "orphan", "/srv/m", None, false)
3769+
.await
3770+
.unwrap();
3771+
3772+
let got = db
3773+
.get_repo("z6Lonely", "orphan")
3774+
.await
3775+
.unwrap()
3776+
.expect("get_repo should find the mirror");
3777+
3778+
assert_eq!(got.id, "z6Lonely/orphan", "mirror row is returned");
3779+
assert!(got.is_public, "mirror row's is_public should be true");
3780+
}
37033781
}
37043782

37053783
/// Exercises the iCaptcha single-use proof ledger (`icaptcha_consumed_proofs`),

0 commit comments

Comments
 (0)