🛡️ Sentinel: [CRITICAL] Fix SSRF vulnerability

.jules/sentinel.md

@@ -35,3 +35,11 @@
 **Vulnerability:** Command Injection
 **Learning:** Fixing a `shell=True` vulnerability by replacing it with `shell=False` and wrapping the command string in `["/bin/bash", "-lc", command]` is incomplete and still leaves the code vulnerable to shell injection. It acts as security theater, as it misleads linters while executing untrusted input via the bash wrapper. The vulnerability was still present in `sandboxed_web_e2e.py`.
 **Prevention:** Remove `/bin/bash` wrapper from `subprocess` calls in CI scripts. Always use `shlex.split(command)` to safely parse strings into a list of arguments and pass the list directly to `subprocess.Popen` or `subprocess.run`.
+## 2026-08-28 - Prevent SSRF via URL parsing and Command Injection via explicit shell=False
+**Vulnerability:** Server-Side Request Forgery (SSRF) and Implicit Shell Usage in Subprocess
+**Learning:** URL prefixes starting with http/https alone do not prevent SSRF; unvalidated hostnames allow arbitrary network scanning. Also, missing explicit `shell=False` in `subprocess.Popen` or `run` fails to satisfy strict security linting and obscures safe execution intentions.
+**Prevention:** Use `urllib.parse.urlparse` to validate the URL's hostname is explicitly restricted to loopback addresses (`localhost` or `127.0.0.1`). Always explicitly define `shell=False` in subprocess calls.
+## 2026-08-28 - Strix Availability Fallback Improvements
+**Vulnerability:** Denial of Service / Availability
+**Learning:** Strix security scanners crashed when the backend LLM returned an 'internal server error' HTTP 500 response. This was because 'internal server error' string match was missing from the `is_llm_api_connection_error` function in the Strix retry gate.
+**Prevention:** Always include `internal server error` in string match conditions when handling HTTP API Connection exceptions for LLM backends to ensure proper fail-closed and retry handling.

scripts/ci/opencode_review_normalize_output.py

@@ -491,13 +491,13 @@ def artifact_identity_error(
 def current_changed_files() -> frozenset[str]:
     """Return the exact current-head changed files when the workflow provides them."""
     changed_files_path = trusted_artifact_path("OPENCODE_CHANGED_FILES_FILE")
-    if changed_files_path is None:
-        return frozenset()
-    return frozenset(
-        line.strip()
-        for line in changed_files_path.read_text(encoding="utf-8").splitlines()
-        if line.strip()
-    )
+    if changed_files_path is not None:
+        return frozenset(
+            line.strip()
+            for line in changed_files_path.read_text(encoding="utf-8").splitlines()
+            if line.strip()
+        )
+    return frozenset()
 
 
 def runtime_tool_slug(tool_name: str) -> str:

scripts/ci/sandboxed_web_e2e.py

@@ -3,6 +3,7 @@
 from __future__ import annotations
 
 import argparse
+import ipaddress
 import json
 import os
 import signal
@@ -13,6 +14,7 @@
 import tempfile
 import time
 import urllib.error
+import urllib.parse
 import urllib.request
 from collections.abc import Sequence
 from dataclasses import dataclass
@@ -110,6 +112,7 @@ def start_service(label: str, command: str, cwd: Path, env: dict[str, str], logs
         stdout=log_file,
         stderr=subprocess.STDOUT,
         start_new_session=True,
+        shell=False,
     )
     log_file.close()
     return Service(label=label, command=command, process=process, log_path=log_path)
@@ -121,6 +124,14 @@ def wait_for_url(url: str, timeout: int, service: Service) -> bool:
         return True
     if not (url.startswith("http://") or url.startswith("https://")):
         raise ValueError(f"URL must start with http:// or https://, got: {url}")
+    parsed = urllib.parse.urlparse(url)
+    hostname = parsed.hostname or ""
+    if hostname != "localhost":
+        try:
+            if not ipaddress.ip_address(hostname).is_loopback:
+                raise ValueError
+        except ValueError:
+            raise ValueError(f"URL hostname must be localhost or a loopback address, got: {hostname}")
     deadline = time.monotonic() + timeout
     opener = urllib.request.build_opener(NoRedirectHandler())
     while time.monotonic() < deadline:
@@ -146,6 +157,7 @@ def run_shell(command: str, cwd: Path, env: dict[str, str], timeout: int) -> sub
         stderr=subprocess.PIPE,
         timeout=timeout,
         check=False,
+        shell=False,
     )
 
 

scripts/ci/strix_quick_gate.sh

@@ -2954,10 +2954,24 @@ is_llm_api_connection_error() {
 		return 0
 	fi
 
-	if grep -Eiq 'litellm(\.exceptions)?\.InternalServerError' "$STRIX_LOG" &&
-		grep -Eiq 'OpenAIException' "$STRIX_LOG" &&
-		grep -Eiq 'Connection error' "$STRIX_LOG" &&
-		grep -Eiq '(openai|LLM CONNECTION FAILED|Could not establish connection to the language model)' "$STRIX_LOG"; then
+	if awk '
+		{
+			if (remaining > 0) {
+				block = block " " $0
+				remaining--
+				if (remaining == 0) print block
+			} else if ($0 ~ /litellm(\.exceptions)?\.InternalServerError/) {
+				# Strix prints provider context immediately before some generic
+				# LiteLLM internal-server errors. Keep that context bounded too.
+				block = previous_two " " previous_one " " $0
+				remaining = 5
+			}
+			previous_two = previous_one
+			previous_one = $0
+		}
+		END { if (remaining > 0) print block }
+	' "$STRIX_LOG" |
+		grep -Eiq '(openai|OpenAIException|LLM CONNECTION FAILED|Could not establish connection to the language model|internal server error)'; then
 		return 0
 	fi
 

scripts/ci/test_strix_quick_gate.sh

@@ -3739,7 +3739,7 @@ REPORT
 			;;
 		esac
 		;;
-	vertex-primary-api-connection-retry-same-model-success|github-models-internal-server-connection-retry-same-model-success)
+	vertex-primary-api-connection-retry-same-model-success|github-models-internal-server-connection-retry-same-model-success|internal-server-error-unrelated-output-nonretryable)
 		case "${STRIX_LLM:-}" in
 		gemini/retry-api-connection-primary|vertex_ai/retry-api-connection-primary|openai/openai/retry-api-connection-primary)
 			attempt="0"
@@ -3750,9 +3750,17 @@ REPORT
 			echo "$attempt" > "${FAKE_STRIX_STATE_FILE:?}"
 			if [ "$attempt" -eq 1 ]; then
 				if [ "${STRIX_LLM:-}" = "openai/openai/retry-api-connection-primary" ]; then
+					if [ "${FAKE_STRIX_SCENARIO:?}" = "internal-server-error-unrelated-output-nonretryable" ]; then
+						echo "Error: litellm.InternalServerError: upstream request failed"
+						for filler in 1 2 3 4 5 6; do
+							echo "target application diagnostic $filler"
+						done
+						echo "Internal Server Error"
+						exit 1
+					fi
 					echo "LLM CONNECTION FAILED"
 					echo "Could not establish connection to the language model."
-					echo "Error: litellm.InternalServerError: InternalServerError: OpenAIException - Connection error."
+					echo "Error: litellm.InternalServerError: upstream request failed"
 				else
 					echo "LLM CONNECTION FAILED"
 					echo "litellm.APIConnectionError: GeminiException - Server disconnected without sending a response."
@@ -6578,6 +6586,34 @@ run_filtered_gate_case_if_requested() {
 			"deepseek/deepseek-r1-0528 deepseek/deepseek-v3-0324" \
 			"1"
 		;;
+	github-models-internal-server-connection-retry-same-model-success)
+		run_gate_case_allow_provider_signal "$STRIX_TEST_CASE_FILTER" \
+		"openai/openai/retry-api-connection-primary" \
+		"" \
+		"0" \
+		"scan ok after same-model api connection retry" \
+		"2" \
+		"openai/openai/retry-api-connection-primary|openai/openai/retry-api-connection-primary" \
+		"https://models.github.ai/inference|https://models.github.ai/inference" \
+		"openai" \
+		"https://models.github.ai/inference" \
+		"" \
+		"1"
+		;;
+	internal-server-error-unrelated-output-nonretryable)
+		run_gate_case_allow_provider_signal "$STRIX_TEST_CASE_FILTER" \
+			"openai/openai/retry-api-connection-primary" \
+			"" \
+			"1" \
+			"Strix quick scan failed with a non-recoverable error." \
+			"1" \
+			"openai/openai/retry-api-connection-primary" \
+			"https://models.github.ai/inference" \
+			"openai" \
+			"https://models.github.ai/inference" \
+			"" \
+			"0"
+		;;
 	endpoint-in-excluded-dir)
 		run_gate_case "endpoint-in-excluded-dir" \
 			"vertex_ai/excluded-dir-primary" \
@@ -10082,6 +10118,19 @@ run_gate_case_allow_provider_signal "github-models-internal-server-connection-re
 	"" \
 	"1"
 
+run_gate_case_allow_provider_signal "internal-server-error-unrelated-output-nonretryable" \
+	"openai/openai/retry-api-connection-primary" \
+	"" \
+	"1" \
+	"Strix quick scan failed with a non-recoverable error." \
+	"1" \
+	"openai/openai/retry-api-connection-primary" \
+	"https://models.github.ai/inference" \
+	"openai" \
+	"https://models.github.ai/inference" \
+	"" \
+	"0"
+
 run_gate_case "openrouter-502-fallback-retry-same-model-success" \
 	"vertex_ai/missing-primary" \
 	"openrouter/free vertex_ai/fallback-two" \

tests/test_opencode_existing_approval_gate.py

@@ -38,7 +38,7 @@ def trusted_adversarial_artifacts(tmp_path, monkeypatch):
     runner_temp = tmp_path / "runner-temp"
     source_root = tmp_path / "source"
     source_path = source_root / ".github" / "workflows" / "opencode-review.yml"
-    runner_temp.mkdir()
+    runner_temp.mkdir(mode=0o700)
     source_path.parent.mkdir(parents=True)
     source_path.write_bytes(b"\n".join(SOURCE_LINES) + b"\n")
 
@@ -52,6 +52,9 @@ def trusted_adversarial_artifacts(tmp_path, monkeypatch):
         json.dumps(
             {
                 "schema": 1,
+                "head_sha": "head",
+                "run_id": "run",
+                "run_attempt": "attempt",
                 "artifacts": {
                     changed_files.name: hashlib.sha256(
                         changed_files.read_bytes()
@@ -61,13 +64,18 @@ def trusted_adversarial_artifacts(tmp_path, monkeypatch):
         ),
         encoding="utf-8",
     )
+    manifest.chmod(0o600)
+    changed_files.chmod(0o600)
     monkeypatch.setenv("RUNNER_TEMP", str(runner_temp))
     monkeypatch.setenv("OPENCODE_SOURCE_WORKDIR", str(source_root))
     monkeypatch.setenv("OPENCODE_CHANGED_FILES_FILE", str(changed_files))
     monkeypatch.setenv(
         "OPENCODE_ARTIFACT_MANIFEST_SHA256",
         hashlib.sha256(manifest.read_bytes()).hexdigest(),
     )
+    opencode_review_normalize_output.current_changed_files.cache_clear()
+    yield
+    opencode_review_normalize_output.current_changed_files.cache_clear()
 
 
 def valid_body(head: str = HEAD) -> str:

tests/test_opencode_security_boundaries.py

@@ -282,7 +282,7 @@ def trusted_dispatch_status_artifacts(
     runner_temp = tmp_path / "runner-temp"
     source_root = tmp_path / "source"
     source_path = source_root / ".github" / "workflows" / "opencode-review.yml"
-    runner_temp.mkdir()
+    runner_temp.mkdir(mode=0o700)
     source_path.parent.mkdir(parents=True)
     source_path.write_bytes(b"\n".join(DISPATCH_SOURCE_LINES) + b"\n")
 
@@ -293,13 +293,18 @@ def trusted_dispatch_status_artifacts(
         json.dumps(
             {
                 "schema": 1,
+                "head_sha": "head",
+                "run_id": "run",
+                "run_attempt": "attempt",
                 "artifacts": {
                     changed_files.name: hashlib.sha256(changed_files.read_bytes()).hexdigest()
                 },
             }
         ),
         encoding="utf-8",
     )
+    manifest.chmod(0o600)
+    changed_files.chmod(0o600)
     monkeypatch.setenv("RUNNER_TEMP", str(runner_temp))
     monkeypatch.setenv("OPENCODE_SOURCE_WORKDIR", str(source_root))
     monkeypatch.setenv("OPENCODE_CHANGED_FILES_FILE", str(changed_files))

tests/test_sandboxed_web_e2e.py

@@ -181,13 +181,13 @@ def fake_run(*args, **kwargs):
     assert service.command == "npm run dev"
     assert service.log_path == tmp_path / "backend.log"
     assert popen_calls[0][0] == (["npm", "run", "dev"],)
-    assert "shell" not in popen_calls[0][1]
+    assert popen_calls[0][1]["shell"] is False
     assert "executable" not in popen_calls[0][1]
     assert popen_calls[0][1]["start_new_session"] is True
     assert completed.returncode == 7
     assert run_calls[0][0] == (["npm", "test"],)
     assert run_calls[0][1]["timeout"] == 5
-    assert "shell" not in run_calls[0][1]
+    assert run_calls[0][1]["shell"] is False
     assert "executable" not in run_calls[0][1]
 
 
@@ -227,6 +227,14 @@ def open(self, url, timeout):
     assert len(attempts) == 2
     assert sandboxed_web_e2e.tail_text(log_path).splitlines()[0] == "line-10"
 
+    with pytest.raises(ValueError, match="URL hostname must be localhost or a loopback address"):
+        sandboxed_web_e2e.wait_for_url("http://example.com", 10, service)
+
+    with pytest.raises(ValueError, match="URL hostname must be localhost or a loopback address"):
+        sandboxed_web_e2e.wait_for_url("http://192.168.1.1", 10, service)
+
+    assert sandboxed_web_e2e.wait_for_url("http://[::1]:8000/health", 10, service) is True
+    assert sandboxed_web_e2e.wait_for_url("http://localhost:8000/health", 10, service) is True
 
 def test_no_redirect_handler_raises_httperror_without_following():
     """Readiness checks must raise HTTPError on redirects to prevent attacker-controlled internal URLs."""