Skip to content

Commit e7c1190

Browse files
nicosuavenicosuave
authored
Fail closed on partial PG auth and remove eval (#136)
1 parent 26e9294 commit e7c1190

6 files changed

Lines changed: 167 additions & 65 deletions

File tree

‎docker-entrypoint.sh‎

Lines changed: 115 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -3,80 +3,134 @@ set -e
33

44
# SIDEMANTIC_MODE: "serve" (default), "mcp", "api", or "both"
55
MODE="${SIDEMANTIC_MODE:-serve}"
6-
DEMO_ARGS=""
7-
if [ -n "$SIDEMANTIC_DEMO" ]; then
8-
DEMO_ARGS="--demo"
9-
fi
106

11-
# Build arg arrays for each command.
12-
# serve accepts: --connection, --db, --host, --port, --username, --password
13-
# mcp-serve accepts: --db only
7+
ENTRYPOINT_ARGS_FILE=$(mktemp)
8+
trap 'rm -f "$ENTRYPOINT_ARGS_FILE"' EXIT
9+
printf '%s\n' "$@" > "$ENTRYPOINT_ARGS_FILE"
1410

15-
# Serve args
16-
SERVE_ARGS="--host 0.0.0.0"
17-
if [ -n "$SIDEMANTIC_CONNECTION" ]; then
18-
SERVE_ARGS="$SERVE_ARGS --connection \"$SIDEMANTIC_CONNECTION\""
19-
fi
20-
if [ -n "$SIDEMANTIC_DB" ]; then
21-
SERVE_ARGS="$SERVE_ARGS --db \"$SIDEMANTIC_DB\""
22-
fi
23-
if [ -n "$SIDEMANTIC_USERNAME" ]; then
24-
SERVE_ARGS="$SERVE_ARGS --username \"$SIDEMANTIC_USERNAME\""
25-
fi
26-
if [ -n "$SIDEMANTIC_PASSWORD" ]; then
27-
SERVE_ARGS="$SERVE_ARGS --password \"$SIDEMANTIC_PASSWORD\""
28-
fi
29-
if [ -n "$SIDEMANTIC_PORT" ]; then
30-
SERVE_ARGS="$SERVE_ARGS --port \"$SIDEMANTIC_PORT\""
31-
fi
11+
run_serve() {
12+
set -- sidemantic serve --host 0.0.0.0
13+
if [ -n "$SIDEMANTIC_CONNECTION" ]; then
14+
set -- "$@" --connection "$SIDEMANTIC_CONNECTION"
15+
fi
16+
if [ -n "$SIDEMANTIC_DB" ]; then
17+
set -- "$@" --db "$SIDEMANTIC_DB"
18+
fi
19+
if [ -n "$SIDEMANTIC_USERNAME" ]; then
20+
set -- "$@" --username "$SIDEMANTIC_USERNAME"
21+
fi
22+
if [ -n "$SIDEMANTIC_PASSWORD" ]; then
23+
set -- "$@" --password "$SIDEMANTIC_PASSWORD"
24+
fi
25+
if [ -n "$SIDEMANTIC_PORT" ]; then
26+
set -- "$@" --port "$SIDEMANTIC_PORT"
27+
fi
28+
if [ -n "$SIDEMANTIC_DEMO" ]; then
29+
set -- "$@" --demo
30+
fi
31+
while IFS= read -r arg; do
32+
set -- "$@" "$arg"
33+
done < "$ENTRYPOINT_ARGS_FILE"
34+
exec "$@"
35+
}
3236

33-
# MCP args (only --db is supported)
34-
MCP_ARGS=""
35-
if [ -n "$SIDEMANTIC_DB" ]; then
36-
MCP_ARGS="$MCP_ARGS --db \"$SIDEMANTIC_DB\""
37-
fi
37+
run_mcp() {
38+
set -- sidemantic mcp-serve
39+
if [ -n "$SIDEMANTIC_DB" ]; then
40+
set -- "$@" --db "$SIDEMANTIC_DB"
41+
fi
42+
if [ -n "$SIDEMANTIC_DEMO" ]; then
43+
set -- "$@" --demo
44+
fi
45+
while IFS= read -r arg; do
46+
set -- "$@" "$arg"
47+
done < "$ENTRYPOINT_ARGS_FILE"
48+
exec "$@"
49+
}
3850

39-
# HTTP API args
40-
API_ARGS="--host 0.0.0.0"
41-
if [ -n "$SIDEMANTIC_CONNECTION" ]; then
42-
API_ARGS="$API_ARGS --connection \"$SIDEMANTIC_CONNECTION\""
43-
fi
44-
if [ -n "$SIDEMANTIC_DB" ]; then
45-
API_ARGS="$API_ARGS --db \"$SIDEMANTIC_DB\""
46-
fi
47-
if [ -n "$SIDEMANTIC_API_TOKEN" ]; then
48-
API_ARGS="$API_ARGS --auth-token \"$SIDEMANTIC_API_TOKEN\""
49-
fi
50-
if [ -n "$SIDEMANTIC_API_PORT" ]; then
51-
API_ARGS="$API_ARGS --port \"$SIDEMANTIC_API_PORT\""
52-
fi
53-
if [ -n "$SIDEMANTIC_MAX_REQUEST_BODY_BYTES" ]; then
54-
API_ARGS="$API_ARGS --max-request-body-bytes \"$SIDEMANTIC_MAX_REQUEST_BODY_BYTES\""
55-
fi
56-
if [ -n "$SIDEMANTIC_CORS_ORIGINS" ]; then
57-
OLD_IFS="$IFS"
58-
IFS=','
59-
for ORIGIN in $SIDEMANTIC_CORS_ORIGINS; do
60-
API_ARGS="$API_ARGS --cors-origin \"$ORIGIN\""
61-
done
62-
IFS="$OLD_IFS"
63-
fi
51+
run_api() {
52+
set -- sidemantic api-serve --host 0.0.0.0
53+
if [ -n "$SIDEMANTIC_CONNECTION" ]; then
54+
set -- "$@" --connection "$SIDEMANTIC_CONNECTION"
55+
fi
56+
if [ -n "$SIDEMANTIC_DB" ]; then
57+
set -- "$@" --db "$SIDEMANTIC_DB"
58+
fi
59+
if [ -n "$SIDEMANTIC_API_TOKEN" ]; then
60+
set -- "$@" --auth-token "$SIDEMANTIC_API_TOKEN"
61+
fi
62+
if [ -n "$SIDEMANTIC_API_PORT" ]; then
63+
set -- "$@" --port "$SIDEMANTIC_API_PORT"
64+
fi
65+
if [ -n "$SIDEMANTIC_MAX_REQUEST_BODY_BYTES" ]; then
66+
set -- "$@" --max-request-body-bytes "$SIDEMANTIC_MAX_REQUEST_BODY_BYTES"
67+
fi
68+
if [ -n "$SIDEMANTIC_CORS_ORIGINS" ]; then
69+
OLD_IFS=$IFS
70+
IFS=','
71+
for ORIGIN in $SIDEMANTIC_CORS_ORIGINS; do
72+
set -- "$@" --cors-origin "$ORIGIN"
73+
done
74+
IFS=$OLD_IFS
75+
fi
76+
if [ -n "$SIDEMANTIC_DEMO" ]; then
77+
set -- "$@" --demo
78+
fi
79+
while IFS= read -r arg; do
80+
set -- "$@" "$arg"
81+
done < "$ENTRYPOINT_ARGS_FILE"
82+
exec "$@"
83+
}
84+
85+
run_both() {
86+
set -- sidemantic serve --host 0.0.0.0
87+
if [ -n "$SIDEMANTIC_CONNECTION" ]; then
88+
set -- "$@" --connection "$SIDEMANTIC_CONNECTION"
89+
fi
90+
if [ -n "$SIDEMANTIC_DB" ]; then
91+
set -- "$@" --db "$SIDEMANTIC_DB"
92+
fi
93+
if [ -n "$SIDEMANTIC_USERNAME" ]; then
94+
set -- "$@" --username "$SIDEMANTIC_USERNAME"
95+
fi
96+
if [ -n "$SIDEMANTIC_PASSWORD" ]; then
97+
set -- "$@" --password "$SIDEMANTIC_PASSWORD"
98+
fi
99+
if [ -n "$SIDEMANTIC_PORT" ]; then
100+
set -- "$@" --port "$SIDEMANTIC_PORT"
101+
fi
102+
if [ -n "$SIDEMANTIC_DEMO" ]; then
103+
set -- "$@" --demo
104+
fi
105+
"$@" &
106+
SERVE_PID=$!
107+
trap 'kill "$SERVE_PID" 2>/dev/null; rm -f "$ENTRYPOINT_ARGS_FILE"' EXIT
108+
109+
set -- sidemantic mcp-serve
110+
if [ -n "$SIDEMANTIC_DB" ]; then
111+
set -- "$@" --db "$SIDEMANTIC_DB"
112+
fi
113+
if [ -n "$SIDEMANTIC_DEMO" ]; then
114+
set -- "$@" --demo
115+
fi
116+
while IFS= read -r arg; do
117+
set -- "$@" "$arg"
118+
done < "$ENTRYPOINT_ARGS_FILE"
119+
exec "$@"
120+
}
64121

65122
case "$MODE" in
66123
serve)
67-
eval exec sidemantic serve $SERVE_ARGS $DEMO_ARGS "$@"
124+
run_serve
68125
;;
69126
mcp)
70-
eval exec sidemantic mcp-serve $MCP_ARGS $DEMO_ARGS "$@"
127+
run_mcp
71128
;;
72129
api)
73-
eval exec sidemantic api-serve $API_ARGS $DEMO_ARGS "$@"
130+
run_api
74131
;;
75132
both)
76-
eval sidemantic serve $SERVE_ARGS $DEMO_ARGS &
77-
SERVE_PID=$!
78-
trap "kill $SERVE_PID 2>/dev/null" EXIT
79-
eval exec sidemantic mcp-serve $MCP_ARGS $DEMO_ARGS "$@"
133+
run_both
80134
;;
81135
*)
82136
echo "Unknown SIDEMANTIC_MODE: $MODE (use serve, mcp, api, or both)" >&2

‎sidemantic/cli.py‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -534,6 +534,10 @@ def serve(
534534
username_resolved = username or (_loaded_config.pg_server.username if _loaded_config else None)
535535
password_resolved = password or (_loaded_config.pg_server.password if _loaded_config else None)
536536

537+
if (username_resolved is None) != (password_resolved is None):
538+
typer.echo("Error: Must provide both --username and --password for PG server auth", err=True)
539+
raise typer.Exit(1)
540+
537541
# Create semantic layer (only pass connection if not None, otherwise use default)
538542
preagg_db = _loaded_config.preagg_database if _loaded_config else None
539543
preagg_sch = _loaded_config.preagg_schema if _loaded_config else None

‎sidemantic/server/connection.py‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,12 +21,14 @@ def __init__(
2121

2222
def handle_auth(self, user, pwd, host, database=None, callback=callable):
2323
"""Handle authentication."""
24-
# If username/password are set, check them
25-
if self.username is not None and self.password is not None:
26-
callback(user == self.username and pwd == self.password)
27-
else:
24+
if self.username is None and self.password is None:
2825
# No auth required
2926
callback(True)
27+
elif self.username is not None and self.password is not None:
28+
callback(user == self.username and pwd == self.password)
29+
else:
30+
# Partial auth config must fail closed.
31+
callback(False)
3032

3133
def handle_connect(self, ip, port, callback=callable):
3234
"""Handle connection."""

‎sidemantic/server/server.py‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,8 @@ def start_server(
4747
username: Username for authentication (optional)
4848
password: Password for authentication (optional)
4949
"""
50+
if (username is None) != (password is None):
51+
raise ValueError("Both username and password must be provided together")
5052

5153
# Create connection class with layer injected
5254
class BoundConnection(SemanticLayerConnection):

‎tests/server/test_connection.py‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,24 @@ def callback(result):
2525
assert auth_results == [True, False]
2626

2727

28+
def test_handle_auth_partial_config_fails_closed():
29+
pytest.importorskip("riffq")
30+
pytest.importorskip("pyarrow")
31+
from sidemantic.server.connection import SemanticLayerConnection
32+
33+
layer = SemanticLayer(connection="duckdb:///:memory:")
34+
conn = SemanticLayerConnection(connection_id=1, executor=None, layer=layer, username="user", password=None)
35+
36+
auth_results = []
37+
38+
def callback(result):
39+
auth_results.append(result)
40+
41+
conn.handle_auth("user", "anything", "localhost", callback=callback)
42+
43+
assert auth_results == [False]
44+
45+
2846
def test_handle_system_queries():
2947
pytest.importorskip("riffq")
3048
pa = pytest.importorskip("pyarrow")

‎tests/test_cli_commands.py‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,21 @@ def fake_start_server(layer, host, port, username, password):
103103
assert called["password"] == "p"
104104

105105

106+
def test_serve_rejects_partial_auth(monkeypatch, tmp_path):
107+
pytest.importorskip("riffq")
108+
109+
def fake_start_server(*args, **kwargs):
110+
raise AssertionError("start_server should not be called with partial auth config")
111+
112+
monkeypatch.setattr("sidemantic.server.server.start_server", fake_start_server)
113+
114+
_write_min_model(tmp_path)
115+
result = runner.invoke(app, ["serve", str(tmp_path), "--username", "u"])
116+
117+
assert result.exit_code == 1
118+
assert "both --username and --password" in result.output
119+
120+
106121
def test_api_serve_calls_start_server(monkeypatch, tmp_path):
107122
pytest.importorskip("fastapi")
108123
called = {}
@@ -176,3 +191,10 @@ def fake_run(*args, **kwargs):
176191
assert called["directory"] == str(tmp_path)
177192
assert called["db_path"] is None
178193
assert called.get("run") is True
194+
195+
196+
def test_docker_entrypoint_does_not_use_eval():
197+
entrypoint_path = Path(__file__).resolve().parent.parent / "docker-entrypoint.sh"
198+
content = entrypoint_path.read_text()
199+
200+
assert "eval " not in content

0 commit comments

Comments
 (0)