Skip to content

add left toolbar and show the graph schema #80

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
gkorland merged 11 commits into from
Aug 21, 2025
Merged

add left toolbar and show the graph schema #80

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
f13439a
add left toolbar
gkorland Aug 19, 2025
a903492
set toolbar
gkorland Aug 20, 2025
a291ef3
add initial schema view
gkorland Aug 20, 2025
4fc9370
show database schema
gkorland Aug 20, 2025
3831b87
add types to column
gkorland Aug 20, 2025
06e15c9
add types to column
gkorland Aug 20, 2025
a41abbb
clean coderabbit comments
gkorland Aug 20, 2025
e85af34
set the graph size
gkorland Aug 20, 2025
7f036ec
Merge branch 'main' into toolbar
gkorland Aug 20, 2025
1ddb0bd
revert size
gkorland Aug 20, 2025
bf70daa
change buttons order
gkorland Aug 20, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
105 changes: 105 additions & 0 deletions api/routes/graphs.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,12 @@
"""Sanitize the query to prevent injection attacks."""
return query.replace('\n', ' ').replace('\r', ' ')[:500]

def sanitize_log_input(value: str) -> str:
"""Sanitize input for safe logging (remove newlines and carriage returns)."""
if not isinstance(value, str):
return str(value)
return value.replace('\n', ' ').replace('\r', ' ')

@graphs_bp.route("")
@token_required
def list_graphs():
Expand All @@ -63,6 +69,105 @@
return jsonify(filtered_graphs)


@graphs_bp.route("/<string:graph_id>/data", methods=["GET"])
@token_required
def get_graph_data(graph_id: str):
"""Return all nodes and edges for the specified graph (namespaced to the user).

This endpoint returns a JSON object with two keys: `nodes` and `edges`.
Nodes contain a minimal set of properties (id, name, labels, props).
Edges contain source and target node names (or internal ids), type and props.
"""
if not graph_id or not isinstance(graph_id, str):
return jsonify({"error": "Invalid graph_id"}), 400

graph_id = graph_id.strip()[:200]
namespaced = g.user_id + "_" + graph_id

try:
graph = db.select_graph(namespaced)
except Exception as e:
logging.error("Failed to select graph %s: %s", sanitize_log_input(namespaced), e)

Check failure

Code scanning / CodeQL

Log Injection High

This log entry depends on a
user-provided value
Loading
.

Copilot Autofix

AI 6 months ago

To fix the log injection vulnerability, we should improve the sanitize_log_input function to remove or escape all control characters that could affect log formatting, not just newlines and carriage returns. This includes tab (\t), form feed (\f), vertical tab (\v), and Unicode line/paragraph separators. The function should also clearly mark user input in the log entry, for example by quoting it. The best way to do this is to use a regular expression to remove all characters in the Unicode "control" category, or to replace them with safe alternatives (such as spaces). The change should be made in the definition of sanitize_log_input in api/routes/graphs.py. No changes to the logging calls themselves are needed if the sanitization is robust.

Required changes:

  • Update the sanitize_log_input function to remove all control characters using a regular expression.
  • Add the required import for re (the regular expression module).
Suggested changeset 1
api/routes/graphs.py

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/api/routes/graphs.py b/api/routes/graphs.py
--- a/api/routes/graphs.py
+++ b/api/routes/graphs.py
@@ -2,6 +2,7 @@
 
 import json
 import logging
+import re
 from concurrent.futures import ThreadPoolExecutor
 from concurrent.futures import TimeoutError as FuturesTimeoutError
 
@@ -50,10 +51,13 @@
     return query.replace('\n', ' ').replace('\r', ' ')[:500]
 
 def sanitize_log_input(value: str) -> str:
-    """Sanitize input for safe logging (remove newlines and carriage returns)."""
+    """Sanitize input for safe logging (remove all control characters and clearly mark user input)."""
     if not isinstance(value, str):
-        return str(value)
-    return value.replace('\n', ' ').replace('\r', ' ')
+        value = str(value)
+    # Remove all Unicode control characters (category Cc)
+    value = re.sub(r'[\x00-\x1F\x7F-\x9F]', ' ', value)
+    # Optionally, quote the user input to make it clear in logs
+    return f'"{value}"'
 
 @graphs_bp.route("")
 @token_required
EOF
@@ -2,6 +2,7 @@

import json
import logging
import re
from concurrent.futures import ThreadPoolExecutor
from concurrent.futures import TimeoutError as FuturesTimeoutError

@@ -50,10 +51,13 @@
return query.replace('\n', ' ').replace('\r', ' ')[:500]

def sanitize_log_input(value: str) -> str:
"""Sanitize input for safe logging (remove newlines and carriage returns)."""
"""Sanitize input for safe logging (remove all control characters and clearly mark user input)."""
if not isinstance(value, str):
return str(value)
return value.replace('\n', ' ').replace('\r', ' ')
value = str(value)
# Remove all Unicode control characters (category Cc)
value = re.sub(r'[\x00-\x1F\x7F-\x9F]', ' ', value)
# Optionally, quote the user input to make it clear in logs
return f'"{value}"'

@graphs_bp.route("")
@token_required
Copilot is powered by AI and may make mistakes. Always verify output.
return jsonify({"error": "Graph not found or database error"}), 404

Comment on lines +87 to +92
Copy link
Contributor

@coderabbitai coderabbitai bot Aug 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Handle missing graph object and harden logging (avoid None deref + log forging finding)

  • db.select_graph can return None; calling graph.query would raise. Return 404 early.
  • Use %r and exc_info=True in logs to mitigate log injection findings and preserve stack traces.
  • Consider 500 for select_graph exceptions (DB errors) vs 404 for “not found”.
     try:
-        graph = db.select_graph(namespaced)
-    except Exception as e:
-        logging.error("Failed to select graph %s: %s", sanitize_log_input(namespaced), e)
-        return jsonify({"error": "Graph not found or database error"}), 404
+        graph = db.select_graph(namespaced)
+    except Exception as e:
+        logging.error("Failed to select graph %r: %s", namespaced, e, exc_info=True)
+        return jsonify({"error": "Database error selecting graph"}), 500
+    if not graph:
+        logging.info("Graph not found: %r", namespaced)
+        return jsonify({"error": "Graph not found"}), 404

Also applies to: 93-106

🧰 Tools
🪛 GitHub Check: CodeQL

[failure] 90-90: Log Injection
This log entry depends on a user-provided value.

🤖 Prompt for AI Agents 
In api/routes/graphs.py around lines 87-92 (and similarly for 93-106),
db.select_graph may return None or raise an exception; update the flow to first
handle a successful return: if graph is None return jsonify({"error":"Graph not
found"}), 404; if select_graph raises an exception treat it as a DB error and
return a 500. Replace the current logging call with a safe log that uses %r for
the namespaced value and include exc_info=True (e.g., logging.error("Failed to
select graph %r", namespaced, exc_info=True)) so stack traces are preserved and
log forging is mitigated, and ensure no graph attributes are accessed before the
None check.

# Build table nodes with columns and table-to-table links (foreign keys)
tables_query = """
MATCH (t:Table)
OPTIONAL MATCH (c:Column)-[:BELONGS_TO]->(t)
RETURN t.name AS table, collect(DISTINCT {name: c.name, type: c.type}) AS columns
"""

links_query = """
MATCH (src_col:Column)-[:BELONGS_TO]->(src_table:Table),
(tgt_col:Column)-[:BELONGS_TO]->(tgt_table:Table),
(src_col)-[:REFERENCES]->(tgt_col)
RETURN DISTINCT src_table.name AS source, tgt_table.name AS target
"""

try:
tables_res = graph.query(tables_query).result_set
links_res = graph.query(links_query).result_set
except Exception as e:
logging.error("Error querying graph data for %s: %s", sanitize_log_input(namespaced), e)

Check failure

Code scanning / CodeQL

Log Injection High

This log entry depends on a
user-provided value
Loading
.

Copilot Autofix

AI 6 months ago

To fix the log injection vulnerability, we should enhance the sanitize_log_input function to remove all control characters (not just newlines and carriage returns) from user input before logging. This can be done by using a regular expression to remove all characters with ASCII codes below 0x20 (except for space), which includes tabs and other control characters. Additionally, we should clearly mark user input in the log entry to prevent confusion. The changes should be made in the sanitize_log_input function in api/routes/graphs.py, and all calls to this function will benefit from the improved sanitization.

Suggested changeset 1
api/routes/graphs.py

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/api/routes/graphs.py b/api/routes/graphs.py
--- a/api/routes/graphs.py
+++ b/api/routes/graphs.py
@@ -49,11 +49,14 @@
     """Sanitize the query to prevent injection attacks."""
     return query.replace('\n', ' ').replace('\r', ' ')[:500]
 
+import re
+
 def sanitize_log_input(value: str) -> str:
-    """Sanitize input for safe logging (remove newlines and carriage returns)."""
+    """Sanitize input for safe logging (remove control characters)."""
     if not isinstance(value, str):
-        return str(value)
-    return value.replace('\n', ' ').replace('\r', ' ')
+        value = str(value)
+    # Remove all ASCII control characters except space (0x20)
+    return re.sub(r'[\x00-\x1F\x7F]', '', value)
 
 @graphs_bp.route("")
 @token_required
EOF
@@ -49,11 +49,14 @@
"""Sanitize the query to prevent injection attacks."""
return query.replace('\n', ' ').replace('\r', ' ')[:500]

import re

def sanitize_log_input(value: str) -> str:
"""Sanitize input for safe logging (remove newlines and carriage returns)."""
"""Sanitize input for safe logging (remove control characters)."""
if not isinstance(value, str):
return str(value)
return value.replace('\n', ' ').replace('\r', ' ')
value = str(value)
# Remove all ASCII control characters except space (0x20)
return re.sub(r'[\x00-\x1F\x7F]', '', value)

@graphs_bp.route("")
@token_required
Copilot is powered by AI and may make mistakes. Always verify output.
return jsonify({"error": "Failed to read graph data"}), 500

nodes = []
for row in tables_res:
try:
table_name, columns = row
except Exception:
continue
# Normalize columns: ensure a list of dicts with name/type
if not isinstance(columns, list):
columns = [] if columns is None else [columns]

normalized = []
for col in columns:
try:
# col may be a mapping-like object or a simple value
if not col:
continue
# Some drivers may return a tuple or list for the collected map
if isinstance(col, (list, tuple)) and len(col) >= 2:
# try to interpret as (name, type)
name = col[0]
ctype = col[1] if len(col) > 1 else None
elif isinstance(col, dict):
name = col.get('name') or col.get('columnName')
ctype = col.get('type') or col.get('dataType')
else:
name = str(col)
ctype = None

if not name:
continue

normalized.append({"name": name, "type": ctype})
except Exception:
continue

nodes.append({
"id": table_name,
"name": table_name,
"columns": normalized,
})

links = []
seen = set()
for row in links_res:
try:
source, target = row
except Exception:
continue
key = (source, target)
if key in seen:
continue
seen.add(key)
links.append({"source": source, "target": target})

return jsonify({"nodes": nodes, "links": links})


@graphs_bp.route("", methods=["POST"])
@token_required
def load_graph():
Expand Down
93 changes: 89 additions & 4 deletions api/static/css/buttons.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,16 +137,15 @@

/* Theme Toggle Button Styles */
#theme-toggle-btn {
position: fixed;
top: 20px;
right: 90px;
position: static;
margin: 0;
}

/* GitHub Link Button Styles */
#github-link-btn {
position: fixed;
top: 20px;
right: 150px;
right: 90px;
display: flex;
align-items: center;
gap: 4px;
Expand Down Expand Up @@ -295,3 +294,89 @@
transform: none;
box-shadow: none;
}

/* Left vertical toolbar (activity bar style) */
/* Left vertical toolbar (activity bar style) */
#left-toolbar {
position: fixed;
top: 0px;
bottom: 0px;
width: 48px;
left: 0px;
background: var(--bg-tertiary, rgba(255,255,255,0.02));
/* border-radius: 10px; */
padding: 6px 6px;
z-index: 1050;
display: flex;
flex-direction: column;
align-items: center;
justify-content: flex-start;
gap: 8px;
box-shadow: 0 6px 18px rgba(0,0,0,0.25);
border: 1px solid var(--border-color, rgba(255,255,255,0.03));
backdrop-filter: blur(6px);
overflow: hidden;
}

#left-toolbar-inner {
display: flex;
flex-direction: column;
align-items: center;
gap: 10px;
width: 100%;
}

.toolbar-button {
width: 40px;
height: 40px;
border-radius: 8px;
display: flex;
align-items: center;
justify-content: center;
box-sizing: border-box;
}
Comment on lines +329 to +337
Copy link
Contributor

@coderabbitai coderabbitai bot Aug 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Improve affordance and focus visibility for toolbar buttons.

Add pointer cursor, normalize border/background, and provide a visible focus indicator for keyboard users.

Apply this diff:

 .toolbar-button {
   width: 40px;
   height: 40px;
   border-radius: 8px;
   display: flex;
   align-items: center;
   justify-content: center;
   box-sizing: border-box;
+  cursor: pointer;
+  border: none;
+  background: transparent;
 }
+
+.toolbar-button:focus-visible {
+  outline: 2px solid var(--falkor-accent);
+  outline-offset: 2px;
+}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
.toolbar-button {
width: 40px;
height: 40px;
border-radius: 8px;
display: flex;
align-items: center;
justify-content: center;
box-sizing: border-box;
}
.toolbar-button {
width: 40px;
height: 40px;
border-radius: 8px;
display: flex;
align-items: center;
justify-content: center;
box-sizing: border-box;
cursor: pointer;
border: none;
background: transparent;
}
.toolbar-button:focus-visible {
outline: 2px solid var(--falkor-accent);
outline-offset: 2px;
}
🤖 Prompt for AI Agents 
In api/static/css/buttons.css around lines 328 to 336, the .toolbar-button lacks
pointer affordance, normalized border/background and a visible keyboard focus
indicator; update the rule to add cursor: pointer, set a neutral background
(e.g. transparent or var(--btn-bg)), and reset border (border: none) while
preserving border-radius and box-sizing, then add a :focus-visible style that
provides a clear, accessible focus ring (for example a 2px solid outline or a
visible box-shadow using a high-contrast focus color and outline-offset) so
keyboard users get a clear indicator without affecting mouse interactions.


.toolbar-button svg { color: var(--text-primary); opacity: 0.95; width:18px; height:18px }

.toolbar-button:hover {
transform: translateY(-4px) scale(1.03);
background: linear-gradient(180deg, rgba(255,255,255,0.02), rgba(255,255,255,0.01));
}

.toolbar-button[aria-pressed="true"] {
background: var(--falkor-accent, rgba(150,120,220,0.22));
transform: translateY(-4px) scale(1.03);
}

#toolbar-buttons {
display: flex;
flex-direction: column;
gap: 8px;
margin-top: 6px;
width: 100%;
align-items: center;
}

#left-toolbar-footer {
margin-top: 8px;
width: 100%;
height: 6px;
flex-shrink: 0;
}

@media (max-width: 768px) {
#left-toolbar {
left: 8px;
top: 12px;
bottom: auto;
width: auto;
height: 48px;
padding: 4px;
border-radius: 8px;
flex-direction: row;
align-items: center;
}
#left-toolbar-inner { flex-direction: row; gap: 6px; }
}


43 changes: 18 additions & 25 deletions api/static/css/menu.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/* Menu and Navigation Components */

#menu-container {
.sidebar-container {
width: 0;
min-width: 0;
overflow: hidden;
Expand All @@ -16,18 +16,14 @@
pointer-events: none;
}

#menu-container.open {
.sidebar-container.open {
width: 30dvw;
padding: 20px;
left: 48px;
visibility: visible;
pointer-events: auto;
}

#menu-header {
display: flex;
flex-direction: row;
}

#menu-content {
flex-grow: 1;
display: flex;
Expand All @@ -36,24 +32,6 @@
gap: 20px;
}

#side-menu-button {
position: absolute;
top: 20px;
left: 20px;
}

#menu-button img {
rotate: 180deg;
filter: brightness(0) invert(1);
}

/* Menu user image styling */
.menu-user-img {
width: 32px;
height: 32px;
object-fit: cover;
}

.menu-item {
overflow: hidden;
flex: 1 1 auto;
Expand Down Expand Up @@ -185,6 +163,21 @@
cursor: pointer;
}

/* Graph canvas container inside schema sidebar */
#schema-graph {
width: 100%;
height: 100%;
min-height: 300px;
background: transparent;
position: relative;
}

/* Ensure canvas inside ForceGraph fills the container */
.force-graph-container, #schema-graph > canvas {
width: 100% !important;
height: 100% !important;
}

#graph-select:focus {
outline: none;
border-color: var(--falkor-border-primary);
Expand Down
21 changes: 4 additions & 17 deletions api/static/css/responsive.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,28 +19,20 @@

/* Menu Responsive */
@media (max-width: 768px) {
#menu-container {
.sidebar-container {
position: fixed;
top: 0;
left: 0;
height: 100vh;
z-index: 999;
}
#menu-container.open {

.sidebar-container.open {
width: 80vw;
padding: 15px;
}
}
Comment on lines 20 to 34
Copy link
Contributor

@coderabbitai coderabbitai bot Aug 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Repo-wide cleanup after selector rename.

To prevent similar regressions, grep for leftover #menu-container, #side-menu-button, and #theme-toggle-btn occurrences across CSS/JS/templates and update/remove them.

🏁 Script executed:

#!/bin/bash
# Find stale selectors/ids after the toolbar/sidebar refactor
set -euo pipefail
echo "Searching for legacy selectors..."
rg -n -S -C2 --glob 'api/**' '#menu-container|#side-menu-button|#theme-toggle-btn'

echo "Check for sidebar open-state rules that may need mobile overrides..."
rg -n -S -C2 --glob 'api/**' '\.sidebar-container\.open'

Length of output: 1955

Cleanup legacy selectors and ensure consistent sidebar styles

The refactor renamed menu and toolbar elements, but the following leftover selectors and duplicate styles remain and should be removed or updated:

  • api/static/js/modules/config.js

    • sideMenuButton: '#side-menu-button'
    • menuContainer: '#menu-container'

  • api/static/css/responsive.css

    • Mobile rule targeting #menu-container.open ~ #chat-container (lines 15–17)

  • api/static/css/buttons.css

    • #theme-toggle-btn selector (lines 139–141)

Additionally, there’s a duplicate .sidebar-container.open rule in api/static/css/menu.css (lines 19–21) with width: 30dvw; padding: 20px; that conflicts with the mobile override in responsive.css. Please remove or consolidate these styles so the sidebar’s open state is governed by a single, consistent media-query rule.

🤖 Prompt for AI Agents 
api/static/css/responsive.css (around lines 20–34) plus related files:
api/static/js/modules/config.js (sideMenuButton/menuContainer),
api/static/css/responsive.css (mobile rule targeting #menu-container.open ~
#chat-container at ~lines 15–17), api/static/css/buttons.css (selector
#theme-toggle-btn at ~lines 139–141), and api/static/css/menu.css (duplicate
.sidebar-container.open at lines 19–21): remove the legacy selectors from
config.js (delete or update sideMenuButton and menuContainer entries), remove
the obsolete #menu-container.open ~ #chat-container mobile rule from
responsive.css, delete the stray #theme-toggle-btn selector from buttons.css,
and remove the duplicate .sidebar-container.open rule in menu.css; then
consolidate the sidebar open-state styling into one media-query rule (keep the
responsive.css mobile rule as the single source of truth—e.g., width: 80vw and
padding: 15px for .sidebar-container.open) so there is no conflicting duplicate
across files.

Comment on lines +30 to 34
Copy link
Contributor

@coderabbitai coderabbitai bot Aug 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Explicitly override desktop open-state offset on mobile.

Cross-file context notes .sidebar-container.open has left: 48px in menu.css. Without overriding here, the drawer may be shifted on mobile. Set left: 0 in the mobile open state to ensure a true overlay.

   .sidebar-container.open {
+    left: 0;
     width: 80vw;
     padding: 15px;
   }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
.sidebar-container.open {
width: 80vw;
padding: 15px;
}
}
.sidebar-container.open {
left: 0;
width: 80vw;
padding: 15px;
}
}
🤖 Prompt for AI Agents 
In api/static/css/responsive.css around lines 30-34, the mobile open-state for
.sidebar-container.open currently sets width and padding but doesn't override
the desktop left: 48px from menu.css; update this mobile rule to set left: 0 for
the .sidebar-container.open selector (and if menu.css uses higher specificity or
inline rules, add !important to the left: 0 declaration) so the drawer becomes a
true full-overlay on mobile.


/* Side Menu Button Responsive */
@media (max-width: 768px) {
#side-menu-button {
top: 15px;
left: 15px;
}
}

/* Chat Messages Responsive */
@media (max-width: 768px) {
.message {
Expand Down Expand Up @@ -129,14 +121,9 @@
height: 40px;
}

#theme-toggle-btn {
top: 15px;
right: 80px;
}

#github-link-btn {
top: 15px;
right: 135px;
right: 60px;
padding: 6px 8px;
font-size: 12px;
height: 40px;
Expand Down
Loading
Loading