diff --git a/docker-entrypoint.sh b/docker-entrypoint.sh index 3528e90..b20b0ef 100644 --- a/docker-entrypoint.sh +++ b/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 diff --git a/sidemantic/cli.py b/sidemantic/cli.py index 31a58f7..3e1f088 100644 --- a/sidemantic/cli.py +++ b/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 diff --git a/sidemantic/server/connection.py b/sidemantic/server/connection.py index 324b7d3..d0d747e 100644 --- a/sidemantic/server/connection.py +++ b/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.""" diff --git a/sidemantic/server/server.py b/sidemantic/server/server.py index 4cd1735..7b227b2 100644 --- a/sidemantic/server/server.py +++ b/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): diff --git a/tests/server/test_connection.py b/tests/server/test_connection.py index 59ae1e7..78b6e5c 100644 --- a/tests/server/test_connection.py +++ b/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") diff --git a/tests/test_cli_commands.py b/tests/test_cli_commands.py index 4159be5..0e9b2ef 100644 --- a/tests/test_cli_commands.py +++ b/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