Fail closed on partial PG auth and remove eval

docker-entrypoint.sh

@@ -3,80 +3,134 @@ set -e
 
 # SIDEMANTIC_MODE: "serve" (default), "mcp", "api", or "both"
 MODE="${SIDEMANTIC_MODE:-serve}"
-DEMO_ARGS=""
-if [ -n "$SIDEMANTIC_DEMO" ]; then
-    DEMO_ARGS="--demo"
-fi
 
-# Build arg arrays for each command.
-# serve accepts: --connection, --db, --host, --port, --username, --password
-# mcp-serve accepts: --db only
+ENTRYPOINT_ARGS_FILE=$(mktemp)
+trap 'rm -f "$ENTRYPOINT_ARGS_FILE"' EXIT
+printf '%s\n' "$@" > "$ENTRYPOINT_ARGS_FILE"
 
-# Serve args
-SERVE_ARGS="--host 0.0.0.0"
-if [ -n "$SIDEMANTIC_CONNECTION" ]; then
-    SERVE_ARGS="$SERVE_ARGS --connection \"$SIDEMANTIC_CONNECTION\""
-fi
-if [ -n "$SIDEMANTIC_DB" ]; then
-    SERVE_ARGS="$SERVE_ARGS --db \"$SIDEMANTIC_DB\""
-fi
-if [ -n "$SIDEMANTIC_USERNAME" ]; then
-    SERVE_ARGS="$SERVE_ARGS --username \"$SIDEMANTIC_USERNAME\""
-fi
-if [ -n "$SIDEMANTIC_PASSWORD" ]; then
-    SERVE_ARGS="$SERVE_ARGS --password \"$SIDEMANTIC_PASSWORD\""
-fi
-if [ -n "$SIDEMANTIC_PORT" ]; then
-    SERVE_ARGS="$SERVE_ARGS --port \"$SIDEMANTIC_PORT\""
-fi
+run_serve() {
+    set -- sidemantic serve --host 0.0.0.0
+    if [ -n "$SIDEMANTIC_CONNECTION" ]; then
+        set -- "$@" --connection "$SIDEMANTIC_CONNECTION"
+    fi
+    if [ -n "$SIDEMANTIC_DB" ]; then
+        set -- "$@" --db "$SIDEMANTIC_DB"
+    fi
+    if [ -n "$SIDEMANTIC_USERNAME" ]; then
+        set -- "$@" --username "$SIDEMANTIC_USERNAME"
+    fi
+    if [ -n "$SIDEMANTIC_PASSWORD" ]; then
+        set -- "$@" --password "$SIDEMANTIC_PASSWORD"
+    fi
+    if [ -n "$SIDEMANTIC_PORT" ]; then
+        set -- "$@" --port "$SIDEMANTIC_PORT"
+    fi
+    if [ -n "$SIDEMANTIC_DEMO" ]; then
+        set -- "$@" --demo
+    fi
+    while IFS= read -r arg; do
+        set -- "$@" "$arg"
+    done < "$ENTRYPOINT_ARGS_FILE"
+    exec "$@"
+}
 
-# MCP args (only --db is supported)
-MCP_ARGS=""
-if [ -n "$SIDEMANTIC_DB" ]; then
-    MCP_ARGS="$MCP_ARGS --db \"$SIDEMANTIC_DB\""
-fi
+run_mcp() {
+    set -- sidemantic mcp-serve
+    if [ -n "$SIDEMANTIC_DB" ]; then
+        set -- "$@" --db "$SIDEMANTIC_DB"
+    fi
+    if [ -n "$SIDEMANTIC_DEMO" ]; then
+        set -- "$@" --demo
+    fi
+    while IFS= read -r arg; do
+        set -- "$@" "$arg"
+    done < "$ENTRYPOINT_ARGS_FILE"
+    exec "$@"
+}
 
-# HTTP API args
-API_ARGS="--host 0.0.0.0"
-if [ -n "$SIDEMANTIC_CONNECTION" ]; then
-    API_ARGS="$API_ARGS --connection \"$SIDEMANTIC_CONNECTION\""
-fi
-if [ -n "$SIDEMANTIC_DB" ]; then
-    API_ARGS="$API_ARGS --db \"$SIDEMANTIC_DB\""
-fi
-if [ -n "$SIDEMANTIC_API_TOKEN" ]; then
-    API_ARGS="$API_ARGS --auth-token \"$SIDEMANTIC_API_TOKEN\""
-fi
-if [ -n "$SIDEMANTIC_API_PORT" ]; then
-    API_ARGS="$API_ARGS --port \"$SIDEMANTIC_API_PORT\""
-fi
-if [ -n "$SIDEMANTIC_MAX_REQUEST_BODY_BYTES" ]; then
-    API_ARGS="$API_ARGS --max-request-body-bytes \"$SIDEMANTIC_MAX_REQUEST_BODY_BYTES\""
-fi
-if [ -n "$SIDEMANTIC_CORS_ORIGINS" ]; then
-    OLD_IFS="$IFS"
-    IFS=','
-    for ORIGIN in $SIDEMANTIC_CORS_ORIGINS; do
-        API_ARGS="$API_ARGS --cors-origin \"$ORIGIN\""
-    done
-    IFS="$OLD_IFS"
-fi
+run_api() {
+    set -- sidemantic api-serve --host 0.0.0.0
+    if [ -n "$SIDEMANTIC_CONNECTION" ]; then
+        set -- "$@" --connection "$SIDEMANTIC_CONNECTION"
+    fi
+    if [ -n "$SIDEMANTIC_DB" ]; then
+        set -- "$@" --db "$SIDEMANTIC_DB"
+    fi
+    if [ -n "$SIDEMANTIC_API_TOKEN" ]; then
+        set -- "$@" --auth-token "$SIDEMANTIC_API_TOKEN"
+    fi
+    if [ -n "$SIDEMANTIC_API_PORT" ]; then
+        set -- "$@" --port "$SIDEMANTIC_API_PORT"
+    fi
+    if [ -n "$SIDEMANTIC_MAX_REQUEST_BODY_BYTES" ]; then
+        set -- "$@" --max-request-body-bytes "$SIDEMANTIC_MAX_REQUEST_BODY_BYTES"
+    fi
+    if [ -n "$SIDEMANTIC_CORS_ORIGINS" ]; then
+        OLD_IFS=$IFS
+        IFS=','
+        for ORIGIN in $SIDEMANTIC_CORS_ORIGINS; do
+            set -- "$@" --cors-origin "$ORIGIN"
+        done
+        IFS=$OLD_IFS
+    fi
+    if [ -n "$SIDEMANTIC_DEMO" ]; then
+        set -- "$@" --demo
+    fi
+    while IFS= read -r arg; do
+        set -- "$@" "$arg"
+    done < "$ENTRYPOINT_ARGS_FILE"
+    exec "$@"
+}
+
+run_both() {
+    set -- sidemantic serve --host 0.0.0.0
+    if [ -n "$SIDEMANTIC_CONNECTION" ]; then
+        set -- "$@" --connection "$SIDEMANTIC_CONNECTION"
+    fi
+    if [ -n "$SIDEMANTIC_DB" ]; then
+        set -- "$@" --db "$SIDEMANTIC_DB"
+    fi
+    if [ -n "$SIDEMANTIC_USERNAME" ]; then
+        set -- "$@" --username "$SIDEMANTIC_USERNAME"
+    fi
+    if [ -n "$SIDEMANTIC_PASSWORD" ]; then
+        set -- "$@" --password "$SIDEMANTIC_PASSWORD"
+    fi
+    if [ -n "$SIDEMANTIC_PORT" ]; then
+        set -- "$@" --port "$SIDEMANTIC_PORT"
+    fi
+    if [ -n "$SIDEMANTIC_DEMO" ]; then
+        set -- "$@" --demo
+    fi
+    "$@" &
+    SERVE_PID=$!
+    trap 'kill "$SERVE_PID" 2>/dev/null; rm -f "$ENTRYPOINT_ARGS_FILE"' EXIT
+
+    set -- sidemantic mcp-serve
+    if [ -n "$SIDEMANTIC_DB" ]; then
+        set -- "$@" --db "$SIDEMANTIC_DB"
+    fi
+    if [ -n "$SIDEMANTIC_DEMO" ]; then
+        set -- "$@" --demo
+    fi
+    while IFS= read -r arg; do
+        set -- "$@" "$arg"
+    done < "$ENTRYPOINT_ARGS_FILE"
+    exec "$@"
+}
 
 case "$MODE" in
     serve)
-        eval exec sidemantic serve $SERVE_ARGS $DEMO_ARGS "$@"
+        run_serve
         ;;
     mcp)
-        eval exec sidemantic mcp-serve $MCP_ARGS $DEMO_ARGS "$@"
+        run_mcp
         ;;
     api)
-        eval exec sidemantic api-serve $API_ARGS $DEMO_ARGS "$@"
+        run_api
         ;;
     both)
-        eval sidemantic serve $SERVE_ARGS $DEMO_ARGS &
-        SERVE_PID=$!
-        trap "kill $SERVE_PID 2>/dev/null" EXIT
-        eval exec sidemantic mcp-serve $MCP_ARGS $DEMO_ARGS "$@"
+        run_both
         ;;
     *)
         echo "Unknown SIDEMANTIC_MODE: $MODE (use serve, mcp, api, or both)" >&2

sidemantic/cli.py

@@ -534,6 +534,10 @@ def serve(
     username_resolved = username or (_loaded_config.pg_server.username if _loaded_config else None)
     password_resolved = password or (_loaded_config.pg_server.password if _loaded_config else None)
 
+    if (username_resolved is None) != (password_resolved is None):
+        typer.echo("Error: Must provide both --username and --password for PG server auth", err=True)
+        raise typer.Exit(1)
+
     # Create semantic layer (only pass connection if not None, otherwise use default)
     preagg_db = _loaded_config.preagg_database if _loaded_config else None
     preagg_sch = _loaded_config.preagg_schema if _loaded_config else None

sidemantic/server/connection.py

@@ -21,12 +21,14 @@ def __init__(
 
     def handle_auth(self, user, pwd, host, database=None, callback=callable):
         """Handle authentication."""
-        # If username/password are set, check them
-        if self.username is not None and self.password is not None:
-            callback(user == self.username and pwd == self.password)
-        else:
+        if self.username is None and self.password is None:
             # No auth required
             callback(True)
+        elif self.username is not None and self.password is not None:
+            callback(user == self.username and pwd == self.password)
+        else:
+            # Partial auth config must fail closed.
+            callback(False)
 
     def handle_connect(self, ip, port, callback=callable):
         """Handle connection."""

sidemantic/server/server.py

@@ -47,6 +47,8 @@ def start_server(
         username: Username for authentication (optional)
         password: Password for authentication (optional)
     """
+    if (username is None) != (password is None):
+        raise ValueError("Both username and password must be provided together")
 
     # Create connection class with layer injected
     class BoundConnection(SemanticLayerConnection):

tests/server/test_connection.py

@@ -25,6 +25,24 @@ def callback(result):
     assert auth_results == [True, False]
 
 
+def test_handle_auth_partial_config_fails_closed():
+    pytest.importorskip("riffq")
+    pytest.importorskip("pyarrow")
+    from sidemantic.server.connection import SemanticLayerConnection
+
+    layer = SemanticLayer(connection="duckdb:///:memory:")
+    conn = SemanticLayerConnection(connection_id=1, executor=None, layer=layer, username="user", password=None)
+
+    auth_results = []
+
+    def callback(result):
+        auth_results.append(result)
+
+    conn.handle_auth("user", "anything", "localhost", callback=callback)
+
+    assert auth_results == [False]
+
+
 def test_handle_system_queries():
     pytest.importorskip("riffq")
     pa = pytest.importorskip("pyarrow")

tests/test_cli_commands.py

@@ -103,6 +103,21 @@ def fake_start_server(layer, host, port, username, password):
     assert called["password"] == "p"
 
 
+def test_serve_rejects_partial_auth(monkeypatch, tmp_path):
+    pytest.importorskip("riffq")
+
+    def fake_start_server(*args, **kwargs):
+        raise AssertionError("start_server should not be called with partial auth config")
+
+    monkeypatch.setattr("sidemantic.server.server.start_server", fake_start_server)
+
+    _write_min_model(tmp_path)
+    result = runner.invoke(app, ["serve", str(tmp_path), "--username", "u"])
+
+    assert result.exit_code == 1
+    assert "both --username and --password" in result.output
+
+
 def test_api_serve_calls_start_server(monkeypatch, tmp_path):
     pytest.importorskip("fastapi")
     called = {}
@@ -176,3 +191,10 @@ def fake_run(*args, **kwargs):
     assert called["directory"] == str(tmp_path)
     assert called["db_path"] is None
     assert called.get("run") is True
+
+
+def test_docker_entrypoint_does_not_use_eval():
+    entrypoint_path = Path(__file__).resolve().parent.parent / "docker-entrypoint.sh"
+    content = entrypoint_path.read_text()
+
+    assert "eval " not in content