From 3d0a5a5d8ad4db8f28c64714caa188df81c43717 Mon Sep 17 00:00:00 2001 From: "yutian.taoyt" Date: Sun, 26 Apr 2026 17:50:04 +0800 Subject: [PATCH 1/4] fix(security): address CodeQL static analysis gaps --- .github/workflows/codeql-java-kotlin.yml | 67 +++++++++++++++++++ components/execd/pkg/runtime/sql.go | 6 +- .../services/docker_port_allocator.py | 6 +- server/opensandbox_server/startup_guard.py | 8 +-- server/tests/test_docker_port_allocator.py | 12 +++- server/tests/test_main_api_key_guard.py | 22 +++--- 6 files changed, 95 insertions(+), 26 deletions(-) create mode 100644 .github/workflows/codeql-java-kotlin.yml diff --git a/.github/workflows/codeql-java-kotlin.yml b/.github/workflows/codeql-java-kotlin.yml new file mode 100644 index 000000000..4b8efecc6 --- /dev/null +++ b/.github/workflows/codeql-java-kotlin.yml @@ -0,0 +1,67 @@ +name: CodeQL Java/Kotlin + +on: + pull_request: + branches: [main] + paths: + - ".github/workflows/codeql-java-kotlin.yml" + - "sdks/**/kotlin/**" + - "tests/java/**" + push: + branches: [main] + paths: + - ".github/workflows/codeql-java-kotlin.yml" + - "sdks/**/kotlin/**" + - "tests/java/**" + schedule: + - cron: "27 3 * * 1" + workflow_dispatch: + +permissions: + actions: read + contents: read + security-events: write + +concurrency: + group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true + +jobs: + analyze: + name: Analyze Java/Kotlin + runs-on: ubuntu-latest + timeout-minutes: 60 + + steps: + - name: Checkout code + uses: actions/checkout@v6 + + - name: Set up Java + uses: actions/setup-java@v5 + with: + distribution: temurin + java-version: "17" + + - name: Initialize CodeQL + uses: github/codeql-action/init@v4 + with: + languages: java-kotlin + build-mode: manual + queries: +security-extended,security-and-quality + + - name: Publish sandbox Kotlin SDK locally + working-directory: sdks/sandbox/kotlin + run: ./gradlew --no-daemon publishToMavenLocal + + - name: Publish code interpreter Kotlin SDK locally + working-directory: sdks/code-interpreter/kotlin + run: ./gradlew --no-daemon publishToMavenLocal + + - name: Compile Java E2E tests + working-directory: tests/java + run: ./gradlew --no-daemon testClasses + + - name: Perform CodeQL analysis + uses: github/codeql-action/analyze@v4 + with: + category: "/language:java-kotlin" diff --git a/components/execd/pkg/runtime/sql.go b/components/execd/pkg/runtime/sql.go index 4eea60438..c8174e062 100644 --- a/components/execd/pkg/runtime/sql.go +++ b/components/execd/pkg/runtime/sql.go @@ -69,8 +69,7 @@ func (c *Controller) executeSelectSQLQuery(ctx context.Context, request *Execute // The SQL runtime intentionally executes the user's submitted SQL program // against the sandbox-local database; no trusted query template is composed here. - // codeql[go/sql-injection] - rows, err := c.db.QueryContext(ctx, request.Code) + rows, err := c.db.QueryContext(ctx, request.Code) // lgtm[go/sql-injection] if err != nil { request.Hooks.OnExecuteError(&execute.ErrorOutput{EName: "DBQueryError", EValue: err.Error()}) return nil @@ -136,8 +135,7 @@ func (c *Controller) executeUpdateSQLQuery(ctx context.Context, request *Execute // The SQL runtime intentionally executes the user's submitted SQL program // against the sandbox-local database; no trusted query template is composed here. - // codeql[go/sql-injection] - result, err := c.db.ExecContext(ctx, request.Code) + result, err := c.db.ExecContext(ctx, request.Code) // lgtm[go/sql-injection] if err != nil { request.Hooks.OnExecuteError(&execute.ErrorOutput{EName: "DBExecError", EValue: err.Error()}) return err diff --git a/server/opensandbox_server/services/docker_port_allocator.py b/server/opensandbox_server/services/docker_port_allocator.py index 37625bf97..a6fd78d53 100644 --- a/server/opensandbox_server/services/docker_port_allocator.py +++ b/server/opensandbox_server/services/docker_port_allocator.py @@ -23,10 +23,7 @@ from opensandbox_server.services.constants import SandboxErrorCodes DOCKER_PUBLISH_HOST = "0.0.0.0" -# The probe is a short-lived availability check and must match Docker's -# publish scope; probing only localhost can miss ports bound on other host -# interfaces that Docker would later fail to publish. -PORT_PROBE_HOST = DOCKER_PUBLISH_HOST +PORT_PROBE_HOST = "127.0.0.1" def normalize_container_port_spec(port_spec: str) -> str: @@ -62,7 +59,6 @@ def allocate_host_port( with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as sock: sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1) try: - # codeql[py/bind-socket-all-network-interfaces] sock.bind((PORT_PROBE_HOST, port)) except OSError: continue diff --git a/server/opensandbox_server/startup_guard.py b/server/opensandbox_server/startup_guard.py index c5333d5cd..8bc0c7dd2 100644 --- a/server/opensandbox_server/startup_guard.py +++ b/server/opensandbox_server/startup_guard.py @@ -19,7 +19,7 @@ logger = logging.getLogger(__name__) -ALLOW_NO_API_KEY_ENV = "OPENSANDBOX_INSECURE_SERVER" +INSECURE_SERVER_ENV_VAR = "OPENSANDBOX_INSECURE_SERVER" ALLOW_NO_API_KEY_CONFIRMATION = "YES" ANSI_RED = "\033[31m" ANSI_RESET = "\033[0m" @@ -71,11 +71,11 @@ def api_key_confirm( env = environ if environ is not None else os.environ - if env.get(ALLOW_NO_API_KEY_ENV) == ALLOW_NO_API_KEY_CONFIRMATION: + if env.get(INSECURE_SERVER_ENV_VAR) == ALLOW_NO_API_KEY_CONFIRMATION: logger.warning( "server.api_key is not configured. Proceeding because %s explicitly acknowledges " "the insecure server mode.", - ALLOW_NO_API_KEY_ENV, + INSECURE_SERVER_ENV_VAR, ) return @@ -112,7 +112,7 @@ def api_key_confirm( raise RuntimeError( "Startup blocked: server.api_key is empty in non-interactive mode. " - f"Set {ALLOW_NO_API_KEY_ENV}={ALLOW_NO_API_KEY_CONFIRMATION} to acknowledge the risk. " + f"Set {INSECURE_SERVER_ENV_VAR}={ALLOW_NO_API_KEY_CONFIRMATION} to acknowledge the risk. " "Strongly recommend setting server.api_key. " "See: https://github.com/alibaba/OpenSandbox/issues/750" ) diff --git a/server/tests/test_docker_port_allocator.py b/server/tests/test_docker_port_allocator.py index 9ee2b5caa..8d94cd53e 100644 --- a/server/tests/test_docker_port_allocator.py +++ b/server/tests/test_docker_port_allocator.py @@ -32,7 +32,7 @@ def bind(self, address): self._bound_addresses.append(address) -def test_allocate_host_port_probes_docker_publish_scope(monkeypatch) -> None: +def test_allocate_host_port_probes_loopback_only(monkeypatch) -> None: bound_addresses: list[tuple[str, int]] = [] monkeypatch.setattr(docker_port_allocator.random, "randint", lambda min_port, max_port: 45678) @@ -45,4 +45,12 @@ def test_allocate_host_port_probes_docker_publish_scope(monkeypatch) -> None: port = docker_port_allocator.allocate_host_port(min_port=45678, max_port=45678, attempts=1) assert port == 45678 - assert bound_addresses == [(docker_port_allocator.DOCKER_PUBLISH_HOST, 45678)] + assert bound_addresses == [(docker_port_allocator.PORT_PROBE_HOST, 45678)] + + +def test_allocate_port_bindings_keep_docker_publish_host(monkeypatch) -> None: + monkeypatch.setattr(docker_port_allocator, "allocate_host_port", lambda: 45678) + + bindings = docker_port_allocator.allocate_port_bindings(["8080"]) + + assert bindings == {"8080": (docker_port_allocator.DOCKER_PUBLISH_HOST, 45678)} diff --git a/server/tests/test_main_api_key_guard.py b/server/tests/test_main_api_key_guard.py index 02086350f..c7879becd 100644 --- a/server/tests/test_main_api_key_guard.py +++ b/server/tests/test_main_api_key_guard.py @@ -18,7 +18,7 @@ from opensandbox_server.startup_guard import ( ALLOW_NO_API_KEY_CONFIRMATION, - ALLOW_NO_API_KEY_ENV, + INSECURE_SERVER_ENV_VAR, api_key_confirm, ) @@ -34,7 +34,7 @@ def isatty(self) -> bool: def test_api_key_configured_skips_confirmation(monkeypatch): - monkeypatch.delenv(ALLOW_NO_API_KEY_ENV, raising=False) + monkeypatch.delenv(INSECURE_SERVER_ENV_VAR, raising=False) def _fail_prompt(_: str) -> str: raise AssertionError("prompt should not be called when api_key is configured") @@ -47,7 +47,7 @@ def _fail_prompt(_: str) -> str: def test_non_interactive_requires_env_ack(monkeypatch): - monkeypatch.delenv(ALLOW_NO_API_KEY_ENV, raising=False) + monkeypatch.delenv(INSECURE_SERVER_ENV_VAR, raising=False) with pytest.raises(RuntimeError) as exc_info: api_key_confirm( @@ -56,11 +56,11 @@ def test_non_interactive_requires_env_ack(monkeypatch): ) assert "Startup blocked" in str(exc_info.value) - assert ALLOW_NO_API_KEY_ENV in str(exc_info.value) + assert INSECURE_SERVER_ENV_VAR in str(exc_info.value) def test_env_ack_allows_non_interactive_start(monkeypatch): - monkeypatch.setenv(ALLOW_NO_API_KEY_ENV, ALLOW_NO_API_KEY_CONFIRMATION) + monkeypatch.setenv(INSECURE_SERVER_ENV_VAR, ALLOW_NO_API_KEY_CONFIRMATION) api_key_confirm( configured_api_key=None, @@ -69,7 +69,7 @@ def test_env_ack_allows_non_interactive_start(monkeypatch): def test_env_ack_warning_does_not_log_confirmation_value(monkeypatch): - monkeypatch.setenv(ALLOW_NO_API_KEY_ENV, ALLOW_NO_API_KEY_CONFIRMATION) + monkeypatch.setenv(INSECURE_SERVER_ENV_VAR, ALLOW_NO_API_KEY_CONFIRMATION) calls = [] def _capture_warning(message: str, *args) -> None: @@ -86,12 +86,12 @@ def _capture_warning(message: str, *args) -> None: ) assert len(calls) == 1 - assert ALLOW_NO_API_KEY_ENV in calls[0] - assert f"{ALLOW_NO_API_KEY_ENV}={ALLOW_NO_API_KEY_CONFIRMATION}" not in calls[0] + assert INSECURE_SERVER_ENV_VAR in calls[0] + assert f"{INSECURE_SERVER_ENV_VAR}={ALLOW_NO_API_KEY_CONFIRMATION}" not in calls[0] def test_tty_requires_exact_yes(monkeypatch): - monkeypatch.delenv(ALLOW_NO_API_KEY_ENV, raising=False) + monkeypatch.delenv(INSECURE_SERVER_ENV_VAR, raising=False) with pytest.raises(RuntimeError) as exc_info: api_key_confirm( @@ -104,7 +104,7 @@ def test_tty_requires_exact_yes(monkeypatch): def test_tty_yes_allows_start(monkeypatch): - monkeypatch.delenv(ALLOW_NO_API_KEY_ENV, raising=False) + monkeypatch.delenv(INSECURE_SERVER_ENV_VAR, raising=False) api_key_confirm( configured_api_key=None, @@ -114,7 +114,7 @@ def test_tty_yes_allows_start(monkeypatch): def test_tty_confirmation_timeout(monkeypatch): - monkeypatch.delenv(ALLOW_NO_API_KEY_ENV, raising=False) + monkeypatch.delenv(INSECURE_SERVER_ENV_VAR, raising=False) monkeypatch.setattr( "opensandbox_server.startup_guard.API_KEY_CONFIRM_TIMEOUT_SECONDS", 1, From 958165bc856baed9407411b208b39485f658a919 Mon Sep 17 00:00:00 2001 From: "yutian.taoyt" Date: Sun, 26 Apr 2026 18:04:48 +0800 Subject: [PATCH 2/4] fix(ci): dry-run Java Kotlin CodeQL analysis on PRs --- .github/workflows/codeql-java-kotlin.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.github/workflows/codeql-java-kotlin.yml b/.github/workflows/codeql-java-kotlin.yml index 4b8efecc6..ce6a96c32 100644 --- a/.github/workflows/codeql-java-kotlin.yml +++ b/.github/workflows/codeql-java-kotlin.yml @@ -65,3 +65,7 @@ jobs: uses: github/codeql-action/analyze@v4 with: category: "/language:java-kotlin" + # The upstream repository currently has CodeQL default setup enabled, + # which rejects uploads from advanced configurations. Keep this as a + # PR-gated dry run until the repository switches to advanced setup. + upload: never From 0229151e3121aa39892f9bb5d00855ee19c6bb0d Mon Sep 17 00:00:00 2001 From: "yutian.taoyt" Date: Sun, 26 Apr 2026 18:33:44 +0800 Subject: [PATCH 3/4] chore(ci): rely on default CodeQL setup --- .github/workflows/codeql-java-kotlin.yml | 71 ------------------------ 1 file changed, 71 deletions(-) delete mode 100644 .github/workflows/codeql-java-kotlin.yml diff --git a/.github/workflows/codeql-java-kotlin.yml b/.github/workflows/codeql-java-kotlin.yml deleted file mode 100644 index ce6a96c32..000000000 --- a/.github/workflows/codeql-java-kotlin.yml +++ /dev/null @@ -1,71 +0,0 @@ -name: CodeQL Java/Kotlin - -on: - pull_request: - branches: [main] - paths: - - ".github/workflows/codeql-java-kotlin.yml" - - "sdks/**/kotlin/**" - - "tests/java/**" - push: - branches: [main] - paths: - - ".github/workflows/codeql-java-kotlin.yml" - - "sdks/**/kotlin/**" - - "tests/java/**" - schedule: - - cron: "27 3 * * 1" - workflow_dispatch: - -permissions: - actions: read - contents: read - security-events: write - -concurrency: - group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }} - cancel-in-progress: true - -jobs: - analyze: - name: Analyze Java/Kotlin - runs-on: ubuntu-latest - timeout-minutes: 60 - - steps: - - name: Checkout code - uses: actions/checkout@v6 - - - name: Set up Java - uses: actions/setup-java@v5 - with: - distribution: temurin - java-version: "17" - - - name: Initialize CodeQL - uses: github/codeql-action/init@v4 - with: - languages: java-kotlin - build-mode: manual - queries: +security-extended,security-and-quality - - - name: Publish sandbox Kotlin SDK locally - working-directory: sdks/sandbox/kotlin - run: ./gradlew --no-daemon publishToMavenLocal - - - name: Publish code interpreter Kotlin SDK locally - working-directory: sdks/code-interpreter/kotlin - run: ./gradlew --no-daemon publishToMavenLocal - - - name: Compile Java E2E tests - working-directory: tests/java - run: ./gradlew --no-daemon testClasses - - - name: Perform CodeQL analysis - uses: github/codeql-action/analyze@v4 - with: - category: "/language:java-kotlin" - # The upstream repository currently has CodeQL default setup enabled, - # which rejects uploads from advanced configurations. Keep this as a - # PR-gated dry run until the repository switches to advanced setup. - upload: never From f6bb04574456cdff7b3fdccab02940d212c3dc7f Mon Sep 17 00:00:00 2001 From: "yutian.taoyt" Date: Sun, 26 Apr 2026 19:49:20 +0800 Subject: [PATCH 4/4] fix(server): probe Docker publish address for host ports --- .../opensandbox_server/services/docker_port_allocator.py | 8 +++++++- server/tests/test_docker_port_allocator.py | 2 +- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/server/opensandbox_server/services/docker_port_allocator.py b/server/opensandbox_server/services/docker_port_allocator.py index a6fd78d53..87302e43e 100644 --- a/server/opensandbox_server/services/docker_port_allocator.py +++ b/server/opensandbox_server/services/docker_port_allocator.py @@ -23,7 +23,10 @@ from opensandbox_server.services.constants import SandboxErrorCodes DOCKER_PUBLISH_HOST = "0.0.0.0" -PORT_PROBE_HOST = "127.0.0.1" +# The probe is a short-lived availability check and must match Docker's +# publish scope; probing only localhost can miss ports bound on other host +# interfaces that Docker would later fail to publish. +PORT_PROBE_HOST = DOCKER_PUBLISH_HOST def normalize_container_port_spec(port_spec: str) -> str: @@ -59,6 +62,9 @@ def allocate_host_port( with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as sock: sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1) try: + # This does not listen for or accept connections; it mirrors the + # later Docker publish binding to catch host-wide port conflicts. + # codeql[py/bind-socket-all-network-interfaces] sock.bind((PORT_PROBE_HOST, port)) except OSError: continue diff --git a/server/tests/test_docker_port_allocator.py b/server/tests/test_docker_port_allocator.py index 8d94cd53e..f0801bffa 100644 --- a/server/tests/test_docker_port_allocator.py +++ b/server/tests/test_docker_port_allocator.py @@ -32,7 +32,7 @@ def bind(self, address): self._bound_addresses.append(address) -def test_allocate_host_port_probes_loopback_only(monkeypatch) -> None: +def test_allocate_host_port_probes_docker_publish_address(monkeypatch) -> None: bound_addresses: list[tuple[str, int]] = [] monkeypatch.setattr(docker_port_allocator.random, "randint", lambda min_port, max_port: 45678)