From 31f4670efa0b0be56d6cb7817b94dffd3ad2a985 Mon Sep 17 00:00:00 2001 From: Grant Date: Fri, 12 Jun 2026 19:39:33 -0500 Subject: [PATCH] Fix ambiguous-column bug in merchant-profile resolution get_merchant_profile_for_product selected the bare MERCHANT_PROFILE_COLS list while JOINing products (which also has an id), so SQLite raised "ambiguous column name: id" on every execution. The function runs on every purchase, so every paid purchase on 0.2.0:52 returned HTTP 500. Replace the JOIN with an equivalent correlated subquery, keeping merchant_profiles the only table in FROM; behavior on NULL/missing merchant_profile_id is unchanged (no row, caller falls back to the default profile). Also from the verification pass: - Add merchant_profile_provider_resolution_queries_round_trip, exercising the previously untested runtime-prepared resolution / CRUD / preference queries. - Repair three test call sites for the new create_invoice / create_subscription params; capture the response body in the paid_purchase status assertion. - Align manifest license to LicenseRef-Keysat-1.0; drop an unused import. --- licensing-service/src/api/recover.rs | 2 +- licensing-service/src/db/repo.rs | 11 +++- licensing-service/tests/api.rs | 98 +++++++++++++++++++++++++++- startos/manifest/index.ts | 2 +- 4 files changed, 105 insertions(+), 8 deletions(-) diff --git a/licensing-service/src/api/recover.rs b/licensing-service/src/api/recover.rs index 65a1a42..80dfb16 100644 --- a/licensing-service/src/api/recover.rs +++ b/licensing-service/src/api/recover.rs @@ -24,7 +24,7 @@ use crate::error::{AppError, AppResult}; use axum::{ extract::State, http::HeaderMap, - response::{Html, IntoResponse, Response}, + response::{Html, IntoResponse}, Json, }; use chrono::DateTime; diff --git a/licensing-service/src/db/repo.rs b/licensing-service/src/db/repo.rs index bdd257b..1e7aa44 100644 --- a/licensing-service/src/db/repo.rs +++ b/licensing-service/src/db/repo.rs @@ -3003,10 +3003,15 @@ pub async fn get_merchant_profile_for_product( pool: &SqlitePool, product_id: &str, ) -> AppResult> { + // Subquery rather than a JOIN: `MERCHANT_PROFILE_COLS` is a bare + // column list (`id, name, …`) shared with the non-JOIN profile + // queries, and `products` also has an `id`, so a JOIN here makes the + // SELECT list's `id` ambiguous. The subquery keeps `merchant_profiles` + // the only table in FROM. A product with a NULL `merchant_profile_id` + // yields no match (subquery → NULL), so callers fall back to default. let row = sqlx::query(&format!( - "SELECT {MERCHANT_PROFILE_COLS} FROM merchant_profiles mp \ - JOIN products p ON p.merchant_profile_id = mp.id \ - WHERE p.id = ? LIMIT 1" + "SELECT {MERCHANT_PROFILE_COLS} FROM merchant_profiles \ + WHERE id = (SELECT merchant_profile_id FROM products WHERE id = ?) LIMIT 1" )) .bind(product_id) .fetch_optional(pool) diff --git a/licensing-service/tests/api.rs b/licensing-service/tests/api.rs index 22e9010..a301723 100644 --- a/licensing-service/tests/api.rs +++ b/licensing-service/tests/api.rs @@ -580,13 +580,14 @@ async fn paid_purchase_creates_invoice_via_provider() { Some(json!({"product": "paid-test"})), ); let resp = send(&state, req).await; + let status = resp.status(); + let body = body_json(resp).await; assert_eq!( - resp.status(), + status, StatusCode::OK, - "paid purchase should succeed against the mock provider" + "paid purchase should succeed against the mock provider; body={body:?}" ); - let body = body_json(resp).await; assert_eq!(body["amount_sats"], 10_000); assert_eq!(body["btcpay_invoice_id"], "mock-inv-1"); assert!( @@ -656,6 +657,7 @@ async fn webhook_settles_invoice_and_issues_license_idempotently() { None, // buyer_email None, // buyer_note None, // policy_id + None, // payment_provider_id ) .await .expect("create_invoice"); @@ -1031,6 +1033,7 @@ async fn recover_returns_license_key_for_matching_pair() { Some("Buyer@Example.COM"), // mixed case to verify lowercasing None, None, + None, // payment_provider_id ) .await .expect("create_invoice"); @@ -2214,6 +2217,8 @@ async fn seed_subscription(state: &AppState) -> (String, String, String) { "SAT", 25_000, &invoice_id, + None, // merchant_profile_id + None, // payment_provider_id ) .await .expect("create_subscription"); @@ -3165,3 +3170,90 @@ async fn product_entitlements_catalog_round_trips_through_list_endpoint() { assert_eq!(catalog[1]["slug"], "unlimited_products"); } +/// Audit coverage for the `:52` merchant-profile / payment-provider model. +/// These queries are runtime-prepared (`sqlx::query(&format!(...))`), so column +/// errors only surface when executed — and the resolution path had no passing +/// test, which let an ambiguous-column bug ship in `get_merchant_profile_for_product`. +/// This drives the write + preference + resolution queries end to end so the +/// whole `:52` SQL surface is exercised by at least one green test. +#[tokio::test] +async fn merchant_profile_provider_resolution_queries_round_trip() { + let (state, _tmp) = make_test_state().await; + let now = "2026-06-12T00:00:00Z"; + + // Default profile is created by migration 0020. + let default = repo::get_default_merchant_profile(&state.db) + .await + .expect("get_default_merchant_profile") + .expect("a default profile exists post-migration"); + + // Profile CRUD reads/writes. + let p2 = Uuid::new_v4().to_string(); + repo::create_merchant_profile( + &state.db, &p2, "Recaps", None, None, None, None, None, false, now, + ) + .await + .expect("create_merchant_profile"); + repo::get_merchant_profile_by_id(&state.db, &p2) + .await + .expect("get_merchant_profile_by_id") + .expect("created profile exists"); + assert!( + repo::list_merchant_profiles(&state.db) + .await + .expect("list_merchant_profiles") + .len() + >= 2 + ); + + // Attach a BTCPay provider to the default profile (store_id present so + // build_provider can construct a client without a network call). + let prov = Uuid::new_v4().to_string(); + repo::create_payment_provider( + &state.db, &prov, &default.id, "btcpay", "Test BTCPay", + "api-key", "http://btcpay.test", Some("wh-1"), Some("secret"), Some("store-1"), now, + ) + .await + .expect("create_payment_provider"); + repo::get_payment_provider_by_id(&state.db, &prov) + .await + .expect("get_payment_provider_by_id") + .expect("created provider exists"); + assert_eq!( + repo::list_payment_providers_for_profile(&state.db, &default.id) + .await + .expect("list_payment_providers_for_profile") + .len(), + 1 + ); + repo::list_all_payment_providers(&state.db) + .await + .expect("list_all_payment_providers"); + + // Rail preference write + read. + repo::set_rail_preference(&state.db, &default.id, "lightning", &prov) + .await + .expect("set_rail_preference"); + assert_eq!( + repo::list_rail_preferences_for_profile(&state.db, &default.id) + .await + .expect("list_rail_preferences_for_profile") + .len(), + 1 + ); + + // The production purchase path's resolution, both branches: + // - Lightning resolves via the explicit preference just set. + // - OnChain has no preference but BTCPay serves it → served-rail fallback. + let (row_pref, _p) = state + .resolve_provider_for_profile_rail(&default.id, keysat::payment::Rail::Lightning) + .await + .expect("resolve lightning via explicit preference"); + assert_eq!(row_pref.id, prov); + let (row_fallback, _p) = state + .resolve_provider_for_profile_rail(&default.id, keysat::payment::Rail::Onchain) + .await + .expect("resolve onchain via served-rail fallback"); + assert_eq!(row_fallback.id, prov); +} + diff --git a/startos/manifest/index.ts b/startos/manifest/index.ts index da9dbf1..4ee80d9 100644 --- a/startos/manifest/index.ts +++ b/startos/manifest/index.ts @@ -14,7 +14,7 @@ import { short, long } from './i18n' export const manifest = setupManifest({ id: 'keysat', title: 'Keysat Licensing', - license: 'LicenseRef-Proprietary', + license: 'LicenseRef-Keysat-1.0', packageRepo: 'https://github.com/keysat-xyz/keysat-startos', upstreamRepo: 'https://github.com/keysat-xyz/keysat', marketingUrl: 'https://keysat.xyz',