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
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ async def _fetch_credential(context: RunnerContext, credential_type: str) -> dic
bot = get_bot_token()
if bot:
req.add_header("Authorization", f"Bearer {bot}")
logger.debug(f"Using CP OIDC token for {credential_type} credentials")

loop = asyncio.get_running_loop()

Expand Down
19 changes: 19 additions & 0 deletions components/runners/ambient-runner/ambient_runner/platform/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,30 @@
# Kubelet automatically refreshes this file when the Secret is updated.
_BOT_TOKEN_FILE = Path("/var/run/secrets/ambient/bot-token")

# K8s SA token mounted in every pod by the kubelet.
_SA_TOKEN_FILE = Path("/var/run/secrets/kubernetes.io/serviceaccount/token")

# In-process cache for the token fetched from the CP token endpoint.
# Set once at startup by _grpc_client.py after a successful CP token fetch.
_cp_fetched_token: str = ""


def get_sa_token() -> str:
"""Return the Kubernetes ServiceAccount token mounted in the pod.
This is a long-lived K8s-managed token that authenticates to the K8s API
as system:serviceaccount:<namespace>:<sa-name>. The backend's
enforceCredentialRBAC classifies this as isBotToken=true, which grants
access to the session owner's credentials without an owner-match check.
"""
try:
if _SA_TOKEN_FILE.exists():
return _SA_TOKEN_FILE.read_text().strip()
except OSError:
pass
return ""


def set_bot_token(token: str) -> None:
"""Store a token fetched from the CP token endpoint for use by get_bot_token()."""
global _cp_fetched_token
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -728,3 +728,91 @@ async def test_returns_success_on_successful_refresh(self):

assert result.get("isError") is None or result.get("isError") is False
assert "successfully" in result["content"][0]["text"].lower()


# ---------------------------------------------------------------------------
# _fetch_credential — CP OIDC token used when no caller token (regression)
# ---------------------------------------------------------------------------


class TestFetchCredentialBotToken:
@pytest.mark.asyncio
async def test_uses_bot_token_when_no_caller_token(self):
"""_fetch_credential sends the CP OIDC token when caller_token is absent.

The api-server validates the CP OIDC token via RHSSO JWT signature verification.
The CP's OIDC client identity must have a role_binding granting credential:read.

Regression for: runner gets HTTP 401 on credential fetch in gRPC-initiated runs.
"""
server = HTTPServer(("127.0.0.1", 0), _CredentialHandler)
port = server.server_address[1]
thread = Thread(target=server.handle_request, daemon=True)
thread.start()

_CredentialHandler.response_body = {"token": "gh-tok-via-oidc"}
_CredentialHandler.captured_headers = {}

cp_oidc_token = "cp-oidc-jwt-token"

try:
with (
patch.dict(
os.environ,
{
"BACKEND_API_URL": f"http://127.0.0.1:{port}/api",
"CREDENTIAL_IDS": json.dumps({"github": "cred-gh-bot-test"}),
},
),
patch("ambient_runner.platform.auth.get_bot_token", return_value=cp_oidc_token),
):
ctx = _make_context() # no caller_token
result = await _fetch_credential(ctx, "github")

assert result.get("token") == "gh-tok-via-oidc", (
"credential fetch must succeed using CP OIDC token — "
"regression for HTTP 401 on gRPC-initiated runs"
)
assert _CredentialHandler.captured_headers.get("Authorization") == (
f"Bearer {cp_oidc_token}"
), "request must use the CP OIDC token"
finally:
server.server_close()
thread.join(timeout=2)

@pytest.mark.asyncio
async def test_bot_token_used_when_no_caller_token(self):
"""CP OIDC token (get_bot_token) is used when caller_token is absent.

The credential endpoint on the api-server validates via RHSSO JWT,
the same issuer that signs the CP OIDC token — one token for both
gRPC and HTTP credential fetches.
"""
called_with = {}

def fake_urlopen(req, timeout=None):
called_with["auth"] = req.get_header("Authorization")
mock_resp = MagicMock()
mock_resp.read.return_value = json.dumps({"token": "ok"}).encode()
mock_resp.__enter__ = lambda s: s
mock_resp.__exit__ = MagicMock(return_value=False)
return mock_resp

with (
patch.dict(
os.environ,
{
"BACKEND_API_URL": "http://backend.svc.cluster.local/api",
"CREDENTIAL_IDS": json.dumps({"github": "cred-gh-pref"}),
},
),
patch("urllib.request.urlopen", side_effect=fake_urlopen),
patch("ambient_runner.platform.auth.get_bot_token", return_value="cp-oidc-token"),
):
ctx = _make_context() # no caller_token
await _fetch_credential(ctx, "github")

assert called_with.get("auth") == "Bearer cp-oidc-token", (
"CP OIDC token must be used for credential fetch — "
"same token used for gRPC and HTTP credential endpoint"
)
Comment on lines +738 to +818
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Tests are asserting the wrong auth source for the no-caller-token path.

Line 740 and Line 784 both enforce CP OIDC/bot token usage when ctx.caller_token is absent. That conflicts with this PR’s intended behavior (Kubernetes ServiceAccount token first, bot token fallback only when SA token is unavailable). As written, these tests either fail against the intended implementation or lock in the old behavior.

Please update these cases to assert:

  1. SA token is used when mounted/available, and
  2. bot token is used only when SA token is unavailable.

As per coding guidelines, "**/*: Flag only errors, security risks, or functionality-breaking problems."

🧰 Tools
🪛 Ruff (0.15.9)

[error] 756-756: Possible hardcoded password assigned to: "cp_oidc_token"

(S105)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@components/runners/ambient-runner/tests/test_shared_session_credentials.py`
around lines 738 - 818, The tests in TestFetchCredentialBotToken are asserting
the bot (CP OIDC) token is used when ctx.caller_token is absent, but the
intended behavior is: prefer a mounted Kubernetes ServiceAccount (SA) token and
fall back to the bot token only if the SA token is unavailable; update
TestFetchCredentialBotToken.test_uses_bot_token_when_no_caller_token and
test_bot_token_used_when_no_caller_token to simulate an available SA token and
assert the Authorization header equals "Bearer <sa-token>" (reference
_fetch_credential, ctx.caller_token, _CredentialHandler.captured_headers, and
fake_urlopen), and add/modify a separate case that removes the SA token
simulation to assert the bot token returned by
ambient_runner.platform.auth.get_bot_token is used as the fallback.

Loading