feat(runner,api,cli): add kubeconfig credential provider for OpenShift MCP auth#1276
Conversation
📝 WalkthroughWalkthroughAdded kubeconfig credential provider support across the platform: extended OpenAPI schema validation to include Changes
Sequence Diagram(s)sequenceDiagram
actor Runner as Ambient Runner
participant CredSvc as Credential Service
participant FS as File System
participant Env as Environment
Runner->>CredSvc: fetch_kubeconfig_credential()
CredSvc-->>Runner: kubeconfig (may include token)
alt token present
Runner->>FS: write token to /tmp/.ambient_kubeconfig
Runner->>FS: set permissions to 0o600
Runner->>Env: set KUBECONFIG=/tmp/.ambient_kubeconfig
Runner->>Runner: log success
else fetch failure or no token
Runner->>Runner: log warning and record auth failure
end
🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/ambient-api-server/openapi/openapi.credentials.yaml`:
- Line 46: The OpenAPI schema lists "kubeconfig" as a provider but the backend
lacks a corresponding ProviderType constant and validation; either add
kubeconfig support or remove it from the schema. To fix: add a new ProviderType
constant "kubeconfig" and include it in the ProviderType.IsValid()
switch/validation logic (and any provider enum/type definitions) so credential
handlers validate it correctly, and implement any provider-specific handling
needed for token/usage paths; alternatively remove "kubeconfig" from the OpenAPI
provider enum entries so the schema matches implemented ProviderType values.
Ensure changes touch the ProviderType constant declarations and the IsValid()
validation method (and propagate to credential handling checks) or update the
OpenAPI enums to remove the token.
In `@components/runners/ambient-runner/.mcp.json`:
- Line 29: Replace the non-deterministic dependency
"kubernetes-mcp-server@latest" with a pinned immutable version (e.g., a specific
semver or exact commit/hash) to avoid pulling unreviewed upstream changes at
runtime; mirror the approach used for "workspace-mcp@1.6.1" by specifying an
exact version for "kubernetes-mcp-server" in the list so the build is
reproducible and auditable.
In `@components/runners/ambient-runner/ambient_runner/platform/auth.py`:
- Line 44: The code currently writes kubeconfig material directly to
_KUBECONFIG_FILE via Path.write_text which is vulnerable to symlink attacks;
change the write sites (the code that writes the kubeconfig content — references
to _KUBECONFIG_FILE) to create a secure temporary file in the same directory
(e.g. tempfile.NamedTemporaryFile(dir=_KUBECONFIG_FILE.parent,
prefix=_KUBECONFIG_FILE.name, delete=False) or use os.open with O_CREAT|O_EXCL),
write the content to that temp file, fsync it, set strict permissions (chmod
0o600), then atomically move it into place using os.replace(_KUBECONFIG_FILE,
target) so the final path is never opened as a symlink; apply this pattern to
both the initial declaration uses and the later write sites that currently call
write_text.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 51530dcb-8c73-487c-bcf1-2a31004c2155
📒 Files selected for processing (4)
components/ambient-api-server/openapi/openapi.credentials.yamlcomponents/ambient-cli/cmd/acpctl/credential/cmd.gocomponents/runners/ambient-runner/.mcp.jsoncomponents/runners/ambient-runner/ambient_runner/platform/auth.py
| "openshift": { | ||
| "command": "uvx", | ||
| "args": [ | ||
| "kubernetes-mcp-server@latest", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify floating MCP package versions in runner MCP configs
fd -i '.mcp.json$' --exec rg -n '"[^"]+@latest"' {}Repository: ambient-code/platform
Length of output: 47
🏁 Script executed:
# Check if the file exists and read its contents
if [ -f "components/runners/ambient-runner/.mcp.json" ]; then
echo "=== File exists, reading contents ==="
cat -n "components/runners/ambient-runner/.mcp.json"
else
echo "=== File does not exist at specified path ==="
# Search for any .mcp.json files
echo "=== Searching for .mcp.json files ==="
find . -name ".mcp.json" -o -name "*.mcp.json" 2>/dev/null | head -20
fiRepository: ambient-code/platform
Length of output: 1776
Pin kubernetes-mcp-server to an immutable version.
Line 29 uses @latest, which pulls unreviewed upstream changes at runtime. This introduces non-deterministic behavior and supply-chain exposure. Compare with line 37's pinned workspace-mcp@1.6.1 approach.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@components/runners/ambient-runner/.mcp.json` at line 29, Replace the
non-deterministic dependency "kubernetes-mcp-server@latest" with a pinned
immutable version (e.g., a specific semver or exact commit/hash) to avoid
pulling unreviewed upstream changes at runtime; mirror the approach used for
"workspace-mcp@1.6.1" by specifying an exact version for "kubernetes-mcp-server"
in the list so the build is reproducible and auditable.
| # time), so updating os.environ mid-run would not reach it without these files. | ||
| _GITHUB_TOKEN_FILE = Path("/tmp/.ambient_github_token") | ||
| _GITLAB_TOKEN_FILE = Path("/tmp/.ambient_gitlab_token") | ||
| _KUBECONFIG_FILE = Path("/tmp/.ambient_kubeconfig") |
There was a problem hiding this comment.
Use symlink-safe file creation for kubeconfig secret material.
Lines 44 and 440-443 write sensitive kubeconfig content to a predictable /tmp path via write_text, which can be abused via symlink attacks in shared environments.
Suggested secure write pattern
- _KUBECONFIG_FILE.write_text(kubeconfig_creds["token"])
- _KUBECONFIG_FILE.chmod(0o600)
+ flags = os.O_WRONLY | os.O_CREAT | os.O_TRUNC
+ if hasattr(os, "O_NOFOLLOW"):
+ flags |= os.O_NOFOLLOW
+ fd = os.open(_KUBECONFIG_FILE, flags, 0o600)
+ with os.fdopen(fd, "w", encoding="utf-8") as f:
+ f.write(kubeconfig_creds["token"])As per coding guidelines, "Flag only errors, security risks, or functionality-breaking problems."
Also applies to: 440-443
🧰 Tools
🪛 Ruff (0.15.9)
[error] 44-44: Probable insecure usage of temporary file or directory: "/tmp/.ambient_kubeconfig"
(S108)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@components/runners/ambient-runner/ambient_runner/platform/auth.py` at line
44, The code currently writes kubeconfig material directly to _KUBECONFIG_FILE
via Path.write_text which is vulnerable to symlink attacks; change the write
sites (the code that writes the kubeconfig content — references to
_KUBECONFIG_FILE) to create a secure temporary file in the same directory (e.g.
tempfile.NamedTemporaryFile(dir=_KUBECONFIG_FILE.parent,
prefix=_KUBECONFIG_FILE.name, delete=False) or use os.open with O_CREAT|O_EXCL),
write the content to that temp file, fsync it, set strict permissions (chmod
0o600), then atomically move it into place using os.replace(_KUBECONFIG_FILE,
target) so the final path is never opened as a symlink; apply this pattern to
both the initial declaration uses and the later write sites that currently call
write_text.
…t MCP auth Enable agents to authenticate to remote OpenShift/Kubernetes clusters via the Credentials API. The runner writes the kubeconfig to a temp file and sets KUBECONFIG so the openshift-mcp-server can connect. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
97af77e to
2ff9963
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
components/runners/ambient-runner/ambient_runner/platform/auth.py (1)
44-44:⚠️ Potential issue | 🔴 CriticalUse symlink-safe, atomic write for kubeconfig secret material.
Line 44uses a predictable/tmptarget andLine 446writes viawrite_text, which can be abused via symlink attacks in shared environments.Proposed fix
+import tempfile @@ _KUBECONFIG_FILE = Path("/tmp/.ambient_kubeconfig") + +def _write_secret_file_atomic(path: Path, content: str) -> None: + path.parent.mkdir(parents=True, exist_ok=True) + fd, tmp_path = tempfile.mkstemp(prefix=f"{path.name}.", dir=path.parent) + try: + os.fchmod(fd, 0o600) + with os.fdopen(fd, "w", encoding="utf-8") as f: + f.write(content) + f.flush() + os.fsync(f.fileno()) + os.replace(tmp_path, path) + path.chmod(0o600) + except Exception: + try: + os.unlink(tmp_path) + except OSError: + pass + raise @@ - _KUBECONFIG_FILE.write_text(kubeconfig_creds["token"]) - _KUBECONFIG_FILE.chmod(0o600) + _write_secret_file_atomic(_KUBECONFIG_FILE, kubeconfig_creds["token"]) os.environ["KUBECONFIG"] = str(_KUBECONFIG_FILE)As per coding guidelines, "Flag only errors, security risks, or functionality-breaking problems."
Also applies to: 446-448
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/runners/ambient-runner/ambient_runner/platform/auth.py` at line 44, The constant _KUBECONFIG_FILE currently points to a predictable /tmp path and the kubeconfig is written with Path.write_text which is vulnerable to symlink attacks; change the write logic (the code that calls write_text for the kubeconfig) to perform a symlink-safe, atomic write: create a NamedTemporaryFile in the same directory, write the kubeconfig bytes to it, fsync the file, set restrictive permissions (e.g., 0o600), close it, then atomically replace _KUBECONFIG_FILE with os.replace(temp_path, _KUBECONFIG_FILE) and ensure no follow of existing symlinks as needed—use these symbols (_KUBECONFIG_FILE and the function/method where write_text is called) to locate and update the code.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@components/runners/ambient-runner/ambient_runner/platform/auth.py`:
- Line 44: The constant _KUBECONFIG_FILE currently points to a predictable /tmp
path and the kubeconfig is written with Path.write_text which is vulnerable to
symlink attacks; change the write logic (the code that calls write_text for the
kubeconfig) to perform a symlink-safe, atomic write: create a NamedTemporaryFile
in the same directory, write the kubeconfig bytes to it, fsync the file, set
restrictive permissions (e.g., 0o600), close it, then atomically replace
_KUBECONFIG_FILE with os.replace(temp_path, _KUBECONFIG_FILE) and ensure no
follow of existing symlinks as needed—use these symbols (_KUBECONFIG_FILE and
the function/method where write_text is called) to locate and update the code.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a74febd6-907d-4869-8292-1fe519f785b6
📒 Files selected for processing (4)
components/ambient-api-server/openapi/openapi.credentials.yamlcomponents/ambient-cli/cmd/acpctl/credential/cmd.gocomponents/runners/ambient-runner/.mcp.jsoncomponents/runners/ambient-runner/ambient_runner/platform/auth.py
✅ Files skipped from review due to trivial changes (2)
- components/ambient-cli/cmd/acpctl/credential/cmd.go
- components/runners/ambient-runner/.mcp.json
markturansky
left a comment
There was a problem hiding this comment.
Nice clean implementation! The kubeconfig credential handling follows the established pattern well.
Observations:
✅ Good stuff:
- Proper file permissions (0o600) on the kubeconfig file
- Clear comment explaining why KUBECONFIG env var is safe to set (doesn't interfere with pod SA)
- Consistent cleanup in
clear_runtime_credentials() - Concurrent credential fetching pattern maintained
Questions/Considerations:
-
File path isolation:
/tmp/.ambient_kubeconfigis a fixed path. If this runner supports concurrent sessions in the same pod, could there be a race condition? (Though looking at the code, it seems like this is per-pod/session, so probably fine) -
MCP server config: The
--disable-multi-clusterflag is interesting - is this intentional for security/scoping reasons, or just simplifying the initial implementation? -
Minor naming inconsistency:
fetch_kubeconfig_credential(singular) vsfetch_github_credentials(plural) - not a blocker, just noticed the pattern difference.
Overall LGTM - the implementation is solid and security-conscious. 🚢
Summary
kubeconfigto the credential provider enum in the OpenAPI spec (4 locations)auth.pyopenshiftMCP server entry (kubernetes-mcp-server) in.mcp.jsonkubeconfigin provider listsAgents can now authenticate to remote OpenShift/Kubernetes clusters via the Credentials API. Users register a kubeconfig credential (
acpctl credential create --provider kubeconfig --token "$(cat ~/.kube/config)"), and the runner writes it to/tmp/.ambient_kubeconfigat session start, settingKUBECONFIGso the openshift-mcp-server can connect.No Go backend changes needed — the credential store is already provider-agnostic. No SDK regeneration needed —
Provideris a plain string field.Test plan
acpctl credential create --provider kubeconfig --token <kubeconfig-yaml>acpctl credential list --provider kubeconfig/tmp/.ambient_kubeconfigand setsKUBECONFIGKUBECONFIGis unset and the file is removed🤖 Generated with Claude Code
Summary by CodeRabbit
kubeconfigas a supported credential provider across API and CLI.