Skip to content

Commit 87d693c

Browse files
logluxloglux
committed
Fix EXPLAIN rendering, quote SQL identifiers, add loading states
- EXPLAIN: iterate over plan rows instead of rendering array as string - Quote all SQL identifiers in migration generators to prevent injection - Add hx-disabled-elt on forms for visual loading feedback - Add CSS for disabled button state
1 parent 6fb5403 commit 87d693c

6 files changed

Lines changed: 41 additions & 24 deletions

File tree

‎app/mcp/tools.py‎

Lines changed: 23 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -175,42 +175,48 @@ def _index_key(index: Dict[str, Any]) -> tuple:
175175
unique = bool(index.get("unique", False))
176176
return (name, cols, unique)
177177

178+
def _qi(identifier: str) -> str:
179+
"""Quote a SQL identifier to prevent injection."""
180+
sanitized = identifier.replace('"', '""')
181+
return f'"{sanitized}"'
182+
178183
def _create_index_sql(dialect: str, name: str, table: str, cols: str, unique: str) -> str:
179-
return f"CREATE {unique}INDEX {name} ON {table} ({cols});"
184+
return f"CREATE {unique}INDEX {_qi(name)} ON {_qi(table)} ({cols});"
180185

181186
def _drop_index_sql(dialect: str, name: str, table: str) -> str:
182187
if dialect == "mysql":
183-
return f"DROP INDEX {name} ON {table};"
184-
return f"DROP INDEX {name};"
188+
return f"DROP INDEX {_qi(name)} ON {_qi(table)};"
189+
return f"DROP INDEX {_qi(name)};"
185190

186191
def _alter_type_sql(
187192
dialect: str, table: str, column: str, desired: str
188193
) -> tuple[str | None, str | None]:
194+
t, c = _qi(table), _qi(column)
189195
if dialect == "postgresql":
190-
return (f"ALTER TABLE {table} ALTER COLUMN {column} TYPE {desired};", None)
196+
return (f"ALTER TABLE {t} ALTER COLUMN {c} TYPE {desired};", None)
191197
if dialect == "mysql":
192-
return (f"ALTER TABLE {table} MODIFY COLUMN {column} {desired};", None)
198+
return (f"ALTER TABLE {t} MODIFY COLUMN {c} {desired};", None)
193199
if dialect == "sqlite":
194200
return (None, f"SQLite does not support ALTER COLUMN TYPE for {table}.{column}")
195-
return (f"ALTER TABLE {table} ALTER COLUMN {column} TYPE {desired};", None)
201+
return (f"ALTER TABLE {t} ALTER COLUMN {c} TYPE {desired};", None)
196202

197203
def _alter_nullable_sql(
198204
dialect: str, table: str, column: str, desired_nullable: bool
199205
) -> tuple[str | None, str | None]:
206+
t, c = _qi(table), _qi(column)
200207
if dialect == "postgresql":
201208
if desired_nullable:
202-
return (f"ALTER TABLE {table} ALTER COLUMN {column} DROP NOT NULL;", None)
203-
return (f"ALTER TABLE {table} ALTER COLUMN {column} SET NOT NULL;", None)
209+
return (f"ALTER TABLE {t} ALTER COLUMN {c} DROP NOT NULL;", None)
210+
return (f"ALTER TABLE {t} ALTER COLUMN {c} SET NOT NULL;", None)
204211
if dialect == "mysql":
205-
# MySQL needs full column definition; warn user.
206212
return (None, f"MySQL requires full column type to change NULL for {table}.{column}")
207213
if dialect == "sqlite":
208214
return (None, f"SQLite does not support ALTER COLUMN NULL for {table}.{column}")
209215
return (None, f"Nullable change not supported for {dialect}")
210216

211217
def _drop_column_sql(dialect: str, table: str, column: str) -> tuple[str | None, str | None]:
212218
if dialect in {"postgresql", "mysql"}:
213-
return (f"ALTER TABLE {table} DROP COLUMN {column};", None)
219+
return (f"ALTER TABLE {_qi(table)} DROP COLUMN {_qi(column)};", None)
214220
if dialect == "sqlite":
215221
return (None, f"SQLite does not support DROP COLUMN for {table}.{column}")
216222
return (None, f"DROP COLUMN not supported for {dialect}")
@@ -289,15 +295,15 @@ def db_migrate_plan(payload: Dict[str, Any]) -> Dict[str, Any]:
289295
for name, c in columns.items():
290296
col_type = c.get("type", "TEXT")
291297
nullable = c.get("nullable", True)
292-
col_defs.append(f"{name} {col_type}{'' if nullable else ' NOT NULL'}")
298+
col_defs.append(f"{_qi(name)} {col_type}{'' if nullable else ' NOT NULL'}")
293299
if col_defs:
294-
statements.append(f"CREATE TABLE {table} ({', '.join(col_defs)});")
300+
statements.append(f"CREATE TABLE {_qi(table)} ({', '.join(col_defs)});")
295301

296302
for idx in spec.get("indexes", []):
297303
idx_norm = _normalize_index(idx)
298304
idx_name = idx_norm.get("name") or f"idx_{table}_{'_'.join(idx_norm['columns'])}"
299305
unique = "UNIQUE " if idx_norm.get("unique") else ""
300-
cols = ", ".join(idx_norm.get("columns", []))
306+
cols = ", ".join(_qi(c) for c in idx_norm.get("columns", []))
301307
if cols:
302308
statements.append(_create_index_sql(dialect, idx_name, table, cols, unique))
303309

@@ -307,7 +313,8 @@ def db_migrate_plan(payload: Dict[str, Any]) -> Dict[str, Any]:
307313
col_type = spec.get("type", "TEXT")
308314
nullable = spec.get("nullable", True)
309315
not_null = "" if nullable else " NOT NULL"
310-
statements.append(f"ALTER TABLE {table} ADD COLUMN {col} {col_type}{not_null};")
316+
stmt = f"ALTER TABLE {_qi(table)} ADD COLUMN {_qi(col)} {col_type}{not_null};"
317+
statements.append(stmt)
311318

312319
for table, cols in diff.get("type_mismatches", {}).items():
313320
for item in cols:
@@ -330,13 +337,13 @@ def db_migrate_plan(payload: Dict[str, Any]) -> Dict[str, Any]:
330337
idx_norm = _normalize_index(idx)
331338
idx_name = idx_norm.get("name") or f"idx_{table}_{'_'.join(idx_norm['columns'])}"
332339
unique = "UNIQUE " if idx_norm.get("unique") else ""
333-
cols = ", ".join(idx_norm.get("columns", []))
340+
cols = ", ".join(_qi(c) for c in idx_norm.get("columns", []))
334341
if cols:
335342
statements.append(_create_index_sql(dialect, idx_name, table, cols, unique))
336343

337344
if destructive:
338345
for table in diff.get("extra_tables", []):
339-
statements.append(f"DROP TABLE {table};")
346+
statements.append(f"DROP TABLE {_qi(table)};")
340347
for table, cols in diff.get("extra_columns", {}).items():
341348
for col in cols:
342349
stmt, warn = _drop_column_sql(dialect, table, col)

‎app/web/static/styles.css‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,13 @@ body {
202202
opacity: 0.85;
203203
}
204204

205+
.button:disabled,
206+
.button[disabled] {
207+
opacity: 0.5;
208+
cursor: not-allowed;
209+
pointer-events: none;
210+
}
211+
205212
.button.ghost {
206213
background: transparent;
207214
border: 1px solid rgb(255 255 255 / 10%);

‎app/web/templates/chat.html‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,8 @@ <h2>{{ active_session.name }}</h2>
8181
{% elif p.command == 'explain' and p.query_result %}
8282
<div class="bubble-label">Explain</div>
8383
{% if p.query_result.plan %}
84-
<pre class="sql-block">{{ p.query_result.plan }}</pre>
84+
<pre class="sql-block">{% for row in p.query_result.plan %}{% for v in row.values() %}{{ v }} {% endfor %}
85+
{% endfor %}</pre>
8586
{% elif p.query_result.error %}
8687
<div class="error">{{ p.query_result.error }}</div>
8788
{% endif %}
@@ -129,6 +130,7 @@ <h2>{{ active_session.name }}</h2>
129130
hx-post="/chat/send"
130131
hx-target="#chat-messages"
131132
hx-swap="beforeend"
133+
hx-disabled-elt="find button"
132134
hx-on::after-request="this.reset(); var el = document.getElementById('chat-messages'); el.scrollTop = el.scrollHeight; var w = document.getElementById('chat-welcome'); if(w) w.remove();">
133135
<input type="hidden" name="session_id" value="{{ session_id }}">
134136
<textarea name="message" id="chat-input" rows="2"

‎app/web/templates/design.html‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
<h2>Design & Diff</h2>
77
<p class="muted">Provide a desired schema JSON to compare with the current database.</p>
88

9-
<form class="form" hx-post="/design/diff" hx-target="#diff-result" hx-swap="innerHTML">
9+
<form class="form" hx-post="/design/diff" hx-target="#diff-result" hx-swap="innerHTML" hx-disabled-elt="find button">
1010
<label for="desired">Desired Schema (JSON)</label>
1111
<textarea id="desired" name="desired" rows="10" placeholder='{"tables": {"users": {"columns": {"id": {"type": "INTEGER", "nullable": false}}}}}'></textarea>
1212
<div class="actions">
@@ -22,7 +22,7 @@ <h2>Design & Diff</h2>
2222
<section class="panel">
2323
<h2>Migration Plan</h2>
2424
<p class="muted">Generate a SQL plan from desired schema (optional destructive).</p>
25-
<form class="form" hx-post="/design/plan" hx-target="#plan-result" hx-swap="innerHTML">
25+
<form class="form" hx-post="/design/plan" hx-target="#plan-result" hx-swap="innerHTML" hx-disabled-elt="find button">
2626
<label for="plan-desired">Desired Schema (JSON)</label>
2727
<textarea id="plan-desired" name="desired" rows="8" placeholder='{"tables": {"users": {"columns": {"id": {"type": "INTEGER", "nullable": false}}}}}'></textarea>
2828
<label class="checkbox">
@@ -42,7 +42,7 @@ <h2>Migration Plan</h2>
4242
<section class="panel">
4343
<h2>Plan & Apply</h2>
4444
<p class="muted">Generate a plan and apply it in one step.</p>
45-
<form class="form" hx-post="/design/plan-apply" hx-target="#plan-apply-result" hx-swap="innerHTML">
45+
<form class="form" hx-post="/design/plan-apply" hx-target="#plan-apply-result" hx-swap="innerHTML" hx-disabled-elt="find button">
4646
<label for="plan-apply-desired">Desired Schema (JSON)</label>
4747
<textarea id="plan-apply-desired" name="desired" rows="8" placeholder='{"tables": {"users": {"columns": {"id": {"type": "INTEGER", "nullable": false}}}}}'></textarea>
4848
<label class="checkbox">
@@ -67,7 +67,7 @@ <h2>Plan & Apply</h2>
6767
<section class="panel">
6868
<h2>Apply SQL</h2>
6969
<p class="muted">Apply a single SQL statement (requires MODE=execute).</p>
70-
<form class="form" hx-post="/design/apply" hx-target="#apply-result" hx-swap="innerHTML">
70+
<form class="form" hx-post="/design/apply" hx-target="#apply-result" hx-swap="innerHTML" hx-disabled-elt="find button">
7171
<label for="apply-sql">SQL Statement</label>
7272
<textarea id="apply-sql" name="sql" rows="6" placeholder="CREATE TABLE ..."></textarea>
7373
<div class="actions">
@@ -84,7 +84,7 @@ <h2>Apply SQL</h2>
8484
<section class="panel">
8585
<h2>Batch Migrate</h2>
8686
<p class="muted">Apply multiple SQL statements, optionally dry-run.</p>
87-
<form class="form" hx-post="/design/migrate" hx-target="#migrate-result" hx-swap="innerHTML">
87+
<form class="form" hx-post="/design/migrate" hx-target="#migrate-result" hx-swap="innerHTML" hx-disabled-elt="find button">
8888
<label for="migrate-sql">SQL Statements (semicolon separated)</label>
8989
<textarea id="migrate-sql" name="sql" rows="8" placeholder="CREATE TABLE ...; ALTER TABLE ...;"></textarea>
9090
<label class="checkbox">

‎app/web/templates/partials/chat_message.html‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,8 @@
4343
{% elif command == 'explain' %}
4444
<div class="bubble-label">Explain</div>
4545
{% if query_result and query_result.plan %}
46-
<pre class="sql-block">{{ query_result.plan }}</pre>
46+
<pre class="sql-block">{% for row in query_result.plan %}{% for v in row.values() %}{{ v }} {% endfor %}
47+
{% endfor %}</pre>
4748
{% elif query_result and query_result.error %}
4849
<div class="error">{{ query_result.error }}</div>
4950
{% else %}

‎app/web/templates/sandbox.html‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
<section class="panel">
66
<h2>Sandbox</h2>
77
<p class="muted">Run SQL queries under the active policy.</p>
8-
<form class="form" hx-post="/sandbox/run" hx-target="#result" hx-swap="innerHTML">
8+
<form class="form" hx-post="/sandbox/run" hx-target="#result" hx-swap="innerHTML" hx-disabled-elt="find button">
99
<label for="sql">SQL Query</label>
1010
<textarea id="sql" name="sql" rows="6" placeholder="SELECT * FROM ..."></textarea>
1111
<div class="actions">

0 commit comments

Comments
 (0)