From 0602655d87410b7d96c67abe6e4a552960c106fc Mon Sep 17 00:00:00 2001 From: Federico Kamelhar Date: Sat, 30 May 2026 07:50:39 -0400 Subject: [PATCH] fix(oracle): honor explicit wallet_password="" for auto-login wallets (closes #289) Pydantic v2 made SecretStr("") falsy, so the truthy guard `if cfg.wallet_password:` silently dropped an explicit wallet_password="" before it reached oracledb.create_pool_async. But "" is python-oracledb's documented idiom for an auto-login (cwallet.sso) wallet. Dropping it forces the driver onto the encrypted ewallet.pem path, which fails at connect time with DPY-6005 / OSError: [Errno 22] (and prompts for a PEM passphrase on a TTY). Switch the guard to `is not None` at all seven Oracle pool builders so an omitted password (None) is still skipped while an explicit "" is forwarded: - memory/backends/oracle.py - memory/backends/oracle_versioned.py - memory/store_backends/oracle.py - rag/embeddings/oracle_indb.py - rag/chunkers/oracle_indb.py - rag/stores/oracle.py - rag/loaders/oracle.py Add tests/unit/test_oracle_wallet_password_empty.py: 7 backends x 3 cases (empty-string forwarded / None omitted / real value forwarded), stubbing oracledb to capture create_pool_async kwargs. Signed-off-by: Federico Kamelhar --- src/locus/memory/backends/oracle.py | 6 +- src/locus/memory/backends/oracle_versioned.py | 4 +- src/locus/memory/store_backends/oracle.py | 4 +- src/locus/rag/chunkers/oracle_indb.py | 4 +- src/locus/rag/embeddings/oracle_indb.py | 4 +- src/locus/rag/loaders/oracle.py | 4 +- src/locus/rag/stores/oracle.py | 4 +- .../unit/test_oracle_wallet_password_empty.py | 152 ++++++++++++++++++ 8 files changed, 175 insertions(+), 7 deletions(-) create mode 100644 tests/unit/test_oracle_wallet_password_empty.py diff --git a/src/locus/memory/backends/oracle.py b/src/locus/memory/backends/oracle.py index a275e36..79feec5 100644 --- a/src/locus/memory/backends/oracle.py +++ b/src/locus/memory/backends/oracle.py @@ -117,7 +117,11 @@ def _build() -> oracledb.AsyncConnectionPool: if self.config.wallet_location: params["config_dir"] = self.config.wallet_location params["wallet_location"] = self.config.wallet_location - if self.config.wallet_password: + # ``is not None`` (not truthiness): SecretStr("") is falsy in + # Pydantic v2, and "" is the python-oracledb idiom for an + # auto-login (cwallet.sso) wallet — a truthy guard would drop + # it and force the encrypted ewallet.pem path. See issue #289. + if self.config.wallet_password is not None: params["wallet_password"] = self.config.wallet_password.get_secret_value() return oracledb.create_pool_async( diff --git a/src/locus/memory/backends/oracle_versioned.py b/src/locus/memory/backends/oracle_versioned.py index 9f2a6ba..a27efa4 100644 --- a/src/locus/memory/backends/oracle_versioned.py +++ b/src/locus/memory/backends/oracle_versioned.py @@ -232,7 +232,9 @@ def _build() -> oracledb.AsyncConnectionPool: if cfg.wallet_location: params["config_dir"] = cfg.wallet_location params["wallet_location"] = cfg.wallet_location - if cfg.wallet_password: + # ``is not None``: SecretStr("") is falsy in Pydantic v2, but + # "" is the auto-login (cwallet.sso) wallet idiom. See #289. + if cfg.wallet_password is not None: params["wallet_password"] = cfg.wallet_password.get_secret_value() return oracledb.create_pool_async( diff --git a/src/locus/memory/store_backends/oracle.py b/src/locus/memory/store_backends/oracle.py index e8b02c2..18d2b1c 100644 --- a/src/locus/memory/store_backends/oracle.py +++ b/src/locus/memory/store_backends/oracle.py @@ -324,7 +324,9 @@ async def _get_pool(self) -> oracledb.AsyncConnectionPool: if cfg.wallet_location: params["config_dir"] = cfg.wallet_location params["wallet_location"] = cfg.wallet_location - if cfg.wallet_password: + # ``is not None``: SecretStr("") is falsy in Pydantic v2, but + # "" is the auto-login (cwallet.sso) wallet idiom. See #289. + if cfg.wallet_password is not None: params["wallet_password"] = cfg.wallet_password.get_secret_value() # create_pool_async returns the pool directly — the "async" diff --git a/src/locus/rag/chunkers/oracle_indb.py b/src/locus/rag/chunkers/oracle_indb.py index 18cf42b..28630e7 100644 --- a/src/locus/rag/chunkers/oracle_indb.py +++ b/src/locus/rag/chunkers/oracle_indb.py @@ -158,7 +158,9 @@ async def _get_pool(self) -> Any: if cfg.wallet_location: params["config_dir"] = cfg.wallet_location params["wallet_location"] = cfg.wallet_location - if cfg.wallet_password: + # ``is not None``: SecretStr("") is falsy in Pydantic v2, but + # "" is the auto-login (cwallet.sso) wallet idiom. See #289. + if cfg.wallet_password is not None: params["wallet_password"] = cfg.wallet_password.get_secret_value() dsn = cfg.dsn diff --git a/src/locus/rag/embeddings/oracle_indb.py b/src/locus/rag/embeddings/oracle_indb.py index e6f0c09..43f9e66 100644 --- a/src/locus/rag/embeddings/oracle_indb.py +++ b/src/locus/rag/embeddings/oracle_indb.py @@ -319,7 +319,9 @@ async def _get_pool(self) -> oracledb.AsyncConnectionPool: if cfg.wallet_location: params["config_dir"] = cfg.wallet_location params["wallet_location"] = cfg.wallet_location - if cfg.wallet_password: + # ``is not None``: SecretStr("") is falsy in Pydantic v2, but + # "" is the auto-login (cwallet.sso) wallet idiom. See #289. + if cfg.wallet_password is not None: params["wallet_password"] = cfg.wallet_password.get_secret_value() self._pool = oracledb.create_pool_async( diff --git a/src/locus/rag/loaders/oracle.py b/src/locus/rag/loaders/oracle.py index d119ab3..604fa35 100644 --- a/src/locus/rag/loaders/oracle.py +++ b/src/locus/rag/loaders/oracle.py @@ -233,7 +233,9 @@ async def _get_pool(self) -> oracledb.AsyncConnectionPool: if self.config.wallet_location: params["config_dir"] = self.config.wallet_location params["wallet_location"] = self.config.wallet_location - if self.config.wallet_password: + # ``is not None``: SecretStr("") is falsy in Pydantic v2, but + # "" is the auto-login (cwallet.sso) wallet idiom. See #289. + if self.config.wallet_password is not None: params["wallet_password"] = self.config.wallet_password.get_secret_value() # create_pool_async returns the pool directly (the "async" diff --git a/src/locus/rag/stores/oracle.py b/src/locus/rag/stores/oracle.py index f359a98..a10cace 100644 --- a/src/locus/rag/stores/oracle.py +++ b/src/locus/rag/stores/oracle.py @@ -321,7 +321,9 @@ def _build() -> oracledb.AsyncConnectionPool: if self.oracle_config.wallet_location: params["config_dir"] = self.oracle_config.wallet_location params["wallet_location"] = self.oracle_config.wallet_location - if self.oracle_config.wallet_password: + # ``is not None``: SecretStr("") is falsy in Pydantic v2, but + # "" is the auto-login (cwallet.sso) wallet idiom. See #289. + if self.oracle_config.wallet_password is not None: params["wallet_password"] = ( self.oracle_config.wallet_password.get_secret_value() ) diff --git a/tests/unit/test_oracle_wallet_password_empty.py b/tests/unit/test_oracle_wallet_password_empty.py new file mode 100644 index 0000000..66c18f7 --- /dev/null +++ b/tests/unit/test_oracle_wallet_password_empty.py @@ -0,0 +1,152 @@ +# Copyright (c) 2025, 2026 Oracle and/or its affiliates. +# Licensed under the Universal Permissive License v1.0 as shown at +# https://oss.oracle.com/licenses/upl/ + +"""Regression tests for issue #289 — empty-string ``wallet_password``. + +In Pydantic v2 ``SecretStr("")`` is *falsy*, so a truthy guard +(``if cfg.wallet_password:``) silently drops an explicit +``wallet_password=""`` before it reaches ``oracledb.create_pool_async``. +But ``""`` is the documented python-oracledb idiom for an auto-login +(``cwallet.sso``) wallet — dropping it forces the driver onto the +encrypted ``ewallet.pem`` path, which fails at connect time with +``DPY-6005`` / ``OSError: [Errno 22]``. + +Every Oracle pool builder in Locus shared this guard. These tests pin +the contract for all of them: an explicit ``""`` is forwarded, an +omitted password (``None``) is not. + +The tests stub ``oracledb`` and capture the kwargs each ``_get_pool`` +sends to ``create_pool_async`` — no real database required. +""" + +from __future__ import annotations + +import sys +import types +from collections.abc import Callable +from typing import Any + +import pytest + + +# --------------------------------------------------------------------------- +# oracledb stub that records create_pool_async kwargs +# --------------------------------------------------------------------------- + + +class _StubPool: + async def close(self) -> None: # pragma: no cover - never acquired here + return None + + +def _install_capturing_oracledb(monkeypatch: pytest.MonkeyPatch) -> list[dict[str, Any]]: + """Stub ``oracledb`` and return a list that collects pool kwargs.""" + calls: list[dict[str, Any]] = [] + + def fake_create_pool_async(*args: Any, **kwargs: Any) -> _StubPool: + calls.append(kwargs) + return _StubPool() + + def fake_makedsn(host: str, port: int, *, service_name: str) -> str: + return f"{host}:{port}/{service_name}" + + fake = types.ModuleType("oracledb") + fake.create_pool_async = fake_create_pool_async # type: ignore[attr-defined] + fake.makedsn = fake_makedsn # type: ignore[attr-defined] + fake.AsyncConnectionPool = _StubPool # type: ignore[attr-defined] + monkeypatch.setitem(sys.modules, "oracledb", fake) + return calls + + +# --------------------------------------------------------------------------- +# Every Locus Oracle pool builder, constructed with a wallet location set +# --------------------------------------------------------------------------- + +_COMMON = {"dsn": "mydb_high", "user": "MYAPP", "password": "pw", "wallet_location": "/w"} # noqa: S106 + + +def _factories() -> list[tuple[str, Callable[..., Any]]]: + """Return (name, builder) pairs. Imports are deferred so the stub is in + place when each module first touches ``oracledb``.""" + from locus.memory.backends.oracle import OracleBackend + from locus.memory.backends.oracle_versioned import OracleCheckpointSaver + from locus.memory.store_backends.oracle import OracleStore + from locus.rag.chunkers.oracle_indb import OracleInDBChunker + from locus.rag.embeddings.oracle_indb import OracleInDBEmbeddings + from locus.rag.loaders.oracle import OracleADBLoader + from locus.rag.stores.oracle import OracleVectorStore + + return [ + ("OracleBackend", lambda **kw: OracleBackend(**_COMMON, **kw)), + ("OracleCheckpointSaver", lambda **kw: OracleCheckpointSaver(**_COMMON, **kw)), + ("OracleStore", lambda **kw: OracleStore(**_COMMON, **kw)), + ( + "OracleInDBEmbeddings", + lambda **kw: OracleInDBEmbeddings(model_name="m", dimension=8, **_COMMON, **kw), + ), + ("OracleInDBChunker", lambda **kw: OracleInDBChunker(**_COMMON, **kw)), + ("OracleVectorStore", lambda **kw: OracleVectorStore(**_COMMON, **kw)), + ( + "OracleADBLoader", + lambda **kw: OracleADBLoader( + sql="SELECT id, body FROM docs", + content_column="body", + **_COMMON, + **kw, + ), + ), + ] + + +def _ids() -> list[str]: + return [name for name, _ in _factories()] + + +@pytest.mark.asyncio +@pytest.mark.parametrize("idx", range(7), ids=_ids()) +async def test_empty_wallet_password_is_forwarded( + idx: int, monkeypatch: pytest.MonkeyPatch +) -> None: + """issue #289: ``wallet_password=""`` must reach create_pool_async.""" + calls = _install_capturing_oracledb(monkeypatch) + _name, build = _factories()[idx] + + backend = build(wallet_password="") + await backend._get_pool() + + assert len(calls) == 1 + assert "wallet_password" in calls[0], ( + "empty-string wallet_password was dropped (truthy-guard regression)" + ) + assert calls[0]["wallet_password"] == "" + + +@pytest.mark.asyncio +@pytest.mark.parametrize("idx", range(7), ids=_ids()) +async def test_omitted_wallet_password_is_not_sent( + idx: int, monkeypatch: pytest.MonkeyPatch +) -> None: + """No wallet password supplied (None) → kwarg must be absent.""" + calls = _install_capturing_oracledb(monkeypatch) + _name, build = _factories()[idx] + + backend = build() # wallet_password defaults to None + await backend._get_pool() + + assert len(calls) == 1 + assert "wallet_password" not in calls[0] + + +@pytest.mark.asyncio +@pytest.mark.parametrize("idx", range(7), ids=_ids()) +async def test_real_wallet_password_is_forwarded(idx: int, monkeypatch: pytest.MonkeyPatch) -> None: + """A non-empty wallet password is forwarded unchanged.""" + calls = _install_capturing_oracledb(monkeypatch) + _name, build = _factories()[idx] + + backend = build(wallet_password="hunter2") # noqa: S106 + await backend._get_pool() + + assert len(calls) == 1 + assert calls[0]["wallet_password"] == "hunter2" # noqa: S105