Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 28 additions & 15 deletions src/splunk_ao/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,25 @@
from splunk_ao.constants import DEFAULT_CONSOLE_URL
from splunk_ao.shared.exceptions import ConfigurationError

# Mapping of SPLUNK_AO_* → GALILEO_* env var pairs used by the bridge.
# Defined at module level so both _bridge_env_vars() and reset() can reference
# the same authoritative list without duplication.
_BRIDGE: list[tuple[str, str]] = [
("SPLUNK_AO_API_KEY", "GALILEO_API_KEY"),
("SPLUNK_AO_API_URL", "GALILEO_API_URL"),
("SPLUNK_AO_CONSOLE_URL", "GALILEO_CONSOLE_URL"),
("SPLUNK_AO_PROJECT", "GALILEO_PROJECT"),
("SPLUNK_AO_PROJECT_ID", "GALILEO_PROJECT_ID"),
("SPLUNK_AO_LOG_STREAM", "GALILEO_LOG_STREAM"),
("SPLUNK_AO_LOG_STREAM_ID", "GALILEO_LOG_STREAM_ID"),
("SPLUNK_AO_JWT_TOKEN", "GALILEO_JWT_TOKEN"),
("SPLUNK_AO_SSO_ID_TOKEN", "GALILEO_SSO_ID_TOKEN"),
("SPLUNK_AO_SSO_PROVIDER", "GALILEO_SSO_PROVIDER"),
("SPLUNK_AO_USERNAME", "GALILEO_USERNAME"),
("SPLUNK_AO_PASSWORD", "GALILEO_PASSWORD"),
("SPLUNK_AO_MODE", "GALILEO_MODE"),
]


class SplunkAOConfig(GalileoConfig):
# Config file for this project.
Expand All @@ -18,6 +37,13 @@ class SplunkAOConfig(GalileoConfig):
_instance: ClassVar[Optional["SplunkAOConfig"]] = None

def reset(self) -> None:
# Remove any GALILEO_* keys the bridge injected into os.environ so that
# the next get() call re-bridges from scratch with whatever SPLUNK_AO_*
# values are current. Without this, galileo-core would re-read the
# stale bridged value after a credential rotation because _bridge_env_vars
# guards against overwriting an already-present GALILEO_* key.
for _splunk_key, galileo_key in _BRIDGE:
os.environ.pop(galileo_key, None)
Comment on lines +45 to +46

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 minor (design): reset() unconditionally pops every GALILEO_* key in _BRIDGE, but _bridge_env_vars() deliberately preserves user-set GALILEO_* values ("explicit GALILEO_* overrides win"). So a user who sets GALILEO_API_KEY directly — without a SPLUNK_AO_* equivalent — will have it silently deleted by reset(), and the next get() will fail with no credential to re-bridge. This breaks the documented override invariant. The PR acknowledges this as a trade-off ("the bridge owns all GALILEO_* keys"), but the safer, symmetric fix is to only clear keys the bridge actually wrote: have _bridge_env_vars() record the keys it set (e.g. a class-level set) and have reset() pop only those. That fixes the stale-bridge bug without touching user-provided overrides. Worth confirming the "no consumer sets GALILEO_* directly" assumption holds for downstream SDKs (splunk-ao-adk / a2a) before relying on it.

🤖 Generated by the Astra agent

SplunkAOConfig._instance = None
super().reset()

Expand All @@ -41,22 +67,9 @@ def _bridge_env_vars() -> None:
so that galileo-core can authenticate successfully.

Only bridges values that are not already set — explicit GALILEO_* overrides win.
reset() clears any previously-bridged GALILEO_* keys so that this guard
cannot return a stale value after a credential rotation.
"""
_BRIDGE = [
("SPLUNK_AO_API_KEY", "GALILEO_API_KEY"),
("SPLUNK_AO_API_URL", "GALILEO_API_URL"),
("SPLUNK_AO_CONSOLE_URL", "GALILEO_CONSOLE_URL"),
("SPLUNK_AO_PROJECT", "GALILEO_PROJECT"),
("SPLUNK_AO_PROJECT_ID", "GALILEO_PROJECT_ID"),
("SPLUNK_AO_LOG_STREAM", "GALILEO_LOG_STREAM"),
("SPLUNK_AO_LOG_STREAM_ID", "GALILEO_LOG_STREAM_ID"),
("SPLUNK_AO_JWT_TOKEN", "GALILEO_JWT_TOKEN"),
("SPLUNK_AO_SSO_ID_TOKEN", "GALILEO_SSO_ID_TOKEN"),
("SPLUNK_AO_SSO_PROVIDER", "GALILEO_SSO_PROVIDER"),
("SPLUNK_AO_USERNAME", "GALILEO_USERNAME"),
("SPLUNK_AO_PASSWORD", "GALILEO_PASSWORD"),
("SPLUNK_AO_MODE", "GALILEO_MODE"),
]
for new_key, old_key in _BRIDGE:
if new_key in os.environ and old_key not in os.environ:
os.environ[old_key] = os.environ[new_key]
Expand Down
98 changes: 97 additions & 1 deletion tests/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

import pytest

from splunk_ao.config import SplunkAOConfig
from splunk_ao.config import _BRIDGE, SplunkAOConfig
from splunk_ao.shared.exceptions import ConfigurationError

# Auth env vars cleared in tests that exercise the missing-auth guard.
Expand Down Expand Up @@ -232,3 +232,99 @@ def test_incomplete_auth_config_rejected_with_specific_guidance(
assert expected_present in message, f"Expected error to reference {expected_present}: {message}"
assert expected_missing in message, f"Expected error to reference {expected_missing}: {message}"
assert "is set but" in message, f"Expected targeted incomplete-config phrasing: {message}"


# ---------------------------------------------------------------------------
# Regression tests: stale bridge after reset() + credential change (HYBIM-787)
# ---------------------------------------------------------------------------


def test_reset_clears_bridged_galileo_env_vars() -> None:
"""reset() must remove every GALILEO_* key the bridge may have written.

Before the fix, galileo-core env vars persisted across reset() calls, so
the _bridge_env_vars guard ("skip if GALILEO_* already set") would silently
reuse the stale value on the next get().

We patch GalileoConfig.reset to a no-op so the test only exercises the
env-var cleanup logic we added, without needing a fully-initialised Pydantic
model instance.
"""
galileo_keys = [galileo_key for _splunk_key, galileo_key in _BRIDGE]

# Seed all GALILEO_* keys to simulate a prior bridge run, and suppress the
# parent reset() so we can isolate the env-var cleanup we added.
with (
patch.dict(os.environ, {k: "stale-value" for k in galileo_keys}, clear=False),
patch("galileo_core.schemas.base_config.GalileoConfig.reset"),
):
mock_instance = MagicMock(spec=SplunkAOConfig)
SplunkAOConfig.reset(mock_instance)

for galileo_key in galileo_keys:
assert galileo_key not in os.environ, (
f"reset() must remove {galileo_key} from os.environ; "
f"found stale value '{os.environ.get(galileo_key)}'"
)


def test_bridge_picks_up_new_credential_after_reset(monkeypatch) -> None:
"""After reset(), _bridge_env_vars must copy the *new* SPLUNK_AO_* value.

Regression test for HYBIM-787: get() → reset() → rotate credential → get()
must result in the updated GALILEO_* value, not the original stale one.

We patch GalileoConfig.reset to a no-op so the test only exercises the
env-var cleanup + re-bridge behaviour without needing a fully-initialised
Pydantic model instance.
"""
monkeypatch.setenv("SPLUNK_AO_API_KEY", "key-first")
monkeypatch.delenv("GALILEO_API_KEY", raising=False)

# First bridge — mirrors key-first into GALILEO_API_KEY.
SplunkAOConfig._bridge_env_vars()
assert os.environ.get("GALILEO_API_KEY") == "key-first", "Bridge must copy key-first on first call"

# reset() — this is the core of the fix under test.
with patch("galileo_core.schemas.base_config.GalileoConfig.reset"):
mock_instance = MagicMock(spec=SplunkAOConfig)
SplunkAOConfig.reset(mock_instance)

assert "GALILEO_API_KEY" not in os.environ, "reset() must remove stale GALILEO_API_KEY"

# Rotate the credential.
monkeypatch.setenv("SPLUNK_AO_API_KEY", "key-rotated")

# Second bridge — must pick up the new key now that reset() cleared the old one.
SplunkAOConfig._bridge_env_vars()
assert os.environ.get("GALILEO_API_KEY") == "key-rotated", (
"After reset() + credential rotation, bridge must copy the new key; "
"got stale value instead"
)
# Cleanup: monkeypatch will restore SPLUNK_AO_API_KEY, but the bridge wrote
# GALILEO_API_KEY directly to os.environ — remove it so it doesn't leak.
os.environ.pop("GALILEO_API_KEY", None)


def test_reset_removes_all_bridgeable_galileo_vars() -> None:
"""reset() removes all GALILEO_* keys the bridge *could* have written.

This is acceptable because no consumer of this SDK sets GALILEO_* directly
(the bridge owns those keys). This test documents the known behaviour so
that any future change to this contract is deliberate and reviewed.
"""
galileo_keys = [galileo_key for _splunk_key, galileo_key in _BRIDGE]

with (
patch.dict(os.environ, {k: "user-set" for k in galileo_keys}, clear=False),
patch("galileo_core.schemas.base_config.GalileoConfig.reset"),
):
mock_instance = MagicMock(spec=SplunkAOConfig)
SplunkAOConfig.reset(mock_instance)

# All GALILEO_* keys are removed — this is the documented trade-off of Option 2.
for galileo_key in galileo_keys:
assert galileo_key not in os.environ, (
f"reset() is expected to remove {galileo_key} "
f"(bridge owns all GALILEO_* keys; no SDK consumer sets them directly)"
)
Loading