From 6b812b60359520fe7da48cb6382b938e387e4b77 Mon Sep 17 00:00:00 2001 From: Justin Ko Date: Sat, 6 Jun 2026 10:17:30 +0900 Subject: [PATCH] fix: GET /v1/user_info 500 on missing broker_credentials table (issue #9) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit trex-core reported GET /v1/user_info returning 500 for valid service/daemon tokens (whoami → daemon_id), degrading the daemon and holding new trades. The reported hypothesis (service-token None deref) was a misdiagnosis. Root cause: app/api/v1_user_info.py: credentials = repo.list_broker_credentials(...) psycopg.errors.UndefinedTable: relation "broker_credentials" does not exist The broker_credentials table was missing and the handler only caught AttributeError, so the DB error propagated to a bare 500. Two fixes: 1. Handler now degrades gracefully when broker-credential lookup fails — returns empty brokers and still serves daemon_id/user_id/permission, which do not depend on broker credentials. This is what trex-core's whoami needs. 2. ensure_schema() now also CREATE TABLE broker_credentials (same shape as migration 20260502_0014). ensure_schema-bootstrapped DBs were missing it, since previously only the alembic path created it. Added tests/test_v1.py regression: broker-credential lookup raising a DB error must yield HTTP 200 with daemon_id, not 500. Verified against the real DB (was 500 → now 200 + daemon_id). Full suite 377 passed. Co-Authored-By: Claude Opus 4.8 --- app/api/v1_user_info.py | 7 +++++++ app/repositories/postgres.py | 13 +++++++++++++ tests/test_v1.py | 31 +++++++++++++++++++++++++++++++ 3 files changed, 51 insertions(+) diff --git a/app/api/v1_user_info.py b/app/api/v1_user_info.py index 79043cc..d2f507f 100644 --- a/app/api/v1_user_info.py +++ b/app/api/v1_user_info.py @@ -59,6 +59,13 @@ def get_user_info( except AttributeError: # repo does not implement list_broker_credentials yet (mock or older impl) pass + except Exception as exc: # pragma: no cover - defensive + # Broker credentials are supplementary. A failure here (e.g. missing + # broker_credentials table, DB error) must not crash /v1/user_info — + # the critical fields (daemon_id, user_id, permission) do not depend + # on broker credentials. Degrade gracefully with an empty broker list. + logger.warning("broker credential lookup failed; returning empty brokers: %s", exc) + brokers = [] finally: if callable(getattr(type(repo), "close", None)): diff --git a/app/repositories/postgres.py b/app/repositories/postgres.py index c87d4bf..00e1911 100644 --- a/app/repositories/postgres.py +++ b/app/repositories/postgres.py @@ -406,6 +406,19 @@ class PostgresPortfolioRepository: ON daemons (owner_user_id) """ ) + cur.execute( + """ + CREATE TABLE IF NOT EXISTS broker_credentials ( + user_id TEXT NOT NULL, + broker TEXT NOT NULL, + account_ref TEXT NOT NULL, + encrypted_payload TEXT NOT NULL, + credential_version INTEGER NOT NULL DEFAULT 1, + last_rotated_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), + PRIMARY KEY (user_id, broker) + ) + """ + ) cur.execute("SELECT EXISTS (SELECT 1 FROM pg_extension WHERE extname = 'timescaledb')") if cur.fetchone()[0]: cur.execute( diff --git a/tests/test_v1.py b/tests/test_v1.py index 63fe667..f84ba6d 100644 --- a/tests/test_v1.py +++ b/tests/test_v1.py @@ -804,6 +804,37 @@ def test_v1_user_info_returns_user(monkeypatch) -> None: assert body["disabled_reason"] is None +def test_v1_user_info_degrades_when_broker_credentials_lookup_fails(monkeypatch) -> None: + """issue #9 regression: broker_credentials lookup failure (missing table / DB error) + must NOT crash /v1/user_info with 500. daemon_id/user_id/permission do not depend on + broker credentials — degrade gracefully with empty brokers.""" + monkeypatch.setenv("PORTFOLIOHUB_DB_ENABLED", "true") + + principal = TokenPrincipal( + token_id="t1", user_id="user-1", permission="read", + token_type="service", runtime_id="rt-1", daemon_id="daemon-9", + ) + user = User(id="user-1", username="alice", permission="read", is_active=True) + + mock_repo = MagicMock() + mock_repo.get_token_principal.return_value = principal + mock_repo.get_user.return_value = user + mock_repo.list_broker_credentials.side_effect = RuntimeError( + 'relation "broker_credentials" does not exist' + ) + + with patch("app.core.auth.get_portfolio_repository_from_settings", return_value=mock_repo), \ + patch("app.api.v1_user_info.get_portfolio_repository_from_settings", return_value=mock_repo): + client = _make_app() + resp = client.get("/v1/user_info", headers=_service_token_header()) + + assert resp.status_code == 200 + body = resp.json() + assert body["daemon_id"] == "daemon-9" + assert body["user_id"] == "user-1" + assert body["brokers"] == [] + + def test_v1_user_info_inactive_user_has_disabled_reason(monkeypatch) -> None: monkeypatch.setenv("PORTFOLIOHUB_DB_ENABLED", "true")