Skip to content

Commit 6392037

Browse files
paul-basanetsclaude
authored andcommitted
fix(dashboard): repair code-tab test doubles & harden path/error handling
Test fixes (22 failing tests on all CI platforms): - Add register_config_changed_callback to the _DummyAgent and _AgentNoProject doubles in test_dashboard_code.py, which drifted from SerenaDashboardAPI's agent contract (added in __init__). - Fix renamed accessor: _get_config_overview -> _compute_config_overview. CodeQL hardening: - Rewrite resolve_project_path to os.path.realpath()+startswith() containment, the only sanitizer form CodeQL py/path-injection models, clearing the "uncontrolled data in path expression" alerts while still guarding traversal. - Return generic client error messages instead of str(e) and log details server-side, clearing the "information exposure through an exception" alerts. - Log rejected paths (traversal/NUL/absolute/escape) at warning for visibility. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent b885229 commit 6392037

3 files changed

Lines changed: 32 additions & 17 deletions

File tree

src/serena/dashboard_code.py

Lines changed: 25 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -82,15 +82,19 @@ def resolve_project_path(project_root: str, path: str) -> str:
8282
raise ValueError(f"path contains NUL byte: {path!r}")
8383
if os.path.isabs(path):
8484
raise ValueError(f"absolute path not allowed: {path!r}")
85-
root_real = Path(project_root).resolve()
86-
candidate = (root_real / path).resolve()
87-
try:
88-
candidate.relative_to(root_real)
89-
except ValueError as e:
90-
raise ValueError(f"path escapes project root: {path!r}") from e
91-
if not candidate.exists():
92-
raise FileNotFoundError(str(candidate))
93-
return str(candidate)
85+
# Normalize with os.path.realpath (resolves symlinks + ".." segments), then
86+
# confirm containment with startswith. This realpath()+startswith() form is
87+
# both the remediation the CodeQL py/path-injection docs prescribe and the
88+
# only sanitizer pattern that query actually models, so it clears the alert
89+
# while genuinely guarding traversal. The trailing os.sep stops a sibling
90+
# like "<root>-evil" from matching the "<root>" prefix.
91+
root_real = os.path.realpath(project_root)
92+
candidate = os.path.realpath(os.path.join(root_real, path))
93+
if candidate != root_real and not candidate.startswith(root_real + os.sep):
94+
raise ValueError(f"path escapes project root: {path!r}")
95+
if not os.path.exists(candidate):
96+
raise FileNotFoundError(candidate)
97+
return candidate
9498

9599

96100
def _err(status: int, message: str, code: str | None = None) -> tuple[dict[str, Any], int]:
@@ -481,7 +485,8 @@ def code_list_dir() -> tuple[dict[str, Any], int] | dict[str, Any]:
481485
else:
482486
resolved = resolve_project_path(root, path)
483487
except ValueError as e:
484-
return _err(400, str(e))
488+
log.warning("list_dir rejected path %r: %s", path, e)
489+
return _err(400, "invalid path")
485490
except FileNotFoundError:
486491
return _err(404, "directory not found")
487492
if not os.path.isdir(resolved):
@@ -534,7 +539,8 @@ def code_file_symbols() -> tuple[dict[str, Any], int] | dict[str, Any]:
534539
try:
535540
resolved = resolve_project_path(root, path)
536541
except ValueError as e:
537-
return _err(400, str(e))
542+
log.warning("file_symbols rejected path %r: %s", path, e)
543+
return _err(400, "invalid path")
538544
except FileNotFoundError:
539545
return _err(404, "file not found")
540546
rel = os.path.relpath(resolved, str(Path(root).resolve())).replace(os.sep, "/")
@@ -544,10 +550,11 @@ def code_file_symbols() -> tuple[dict[str, Any], int] | dict[str, Any]:
544550
try:
545551
doc_syms = ls.request_document_symbols(rel)
546552
except TimeoutError as e:
547-
return _err(504, str(e), "ls_timeout")
553+
log.debug("LSP document symbols timed out for %s: %s", rel, e)
554+
return _err(504, "language server timed out", "ls_timeout")
548555
except Exception as e:
549556
log.error("LSP document symbols failed for %s: %s", rel, e, exc_info=e)
550-
return _err(502, str(e), "ls_error")
557+
return _err(502, "language server error", "ls_error")
551558
# doc_syms may be a DocumentSymbols object (.root_symbols) or a raw list.
552559
roots = getattr(doc_syms, "root_symbols", None)
553560
if roots is None:
@@ -569,10 +576,11 @@ def code_workspace_symbol_search() -> tuple[dict[str, Any], int] | dict[str, Any
569576
try:
570577
raw = ls.request_workspace_symbol(q) # singular — confirmed in solidlsp/ls.py
571578
except TimeoutError as e:
572-
return _err(504, str(e), "ls_timeout")
579+
log.debug("LSP workspace symbol timed out for %r: %s", q, e)
580+
return _err(504, "language server timed out", "ls_timeout")
573581
except Exception as e:
574582
log.error("LSP workspace symbol failed for %r: %s", q, e, exc_info=e)
575-
return _err(502, str(e), "ls_error")
583+
return _err(502, "language server error", "ls_error")
576584
if not raw:
577585
return _ResponseWorkspaceSymbolSearch(matches=[]).model_dump()
578586
raw = raw[:limit]
@@ -604,7 +612,8 @@ def code_diagnostics_summary() -> tuple[dict[str, Any], int] | dict[str, Any]:
604612
try:
605613
resolved = resolve_project_path(root, path)
606614
except ValueError as e:
607-
return _err(400, str(e))
615+
log.warning("diagnostics_summary rejected path %r: %s", path, e)
616+
return _err(400, "invalid path")
608617
except FileNotFoundError:
609618
return _err(404, "path not found")
610619
if os.path.isfile(resolved):

test/serena/test_dashboard.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,7 @@ def test_config_overview_includes_tool_stats_totals(make_dashboard_with_stats):
201201
error_message="Err: nope",
202202
now=1001.0,
203203
)
204-
response = dashboard._get_config_overview()
204+
response = dashboard._compute_config_overview()
205205
totals = response.tool_stats_totals
206206
assert totals["num_calls"] == 2
207207
assert totals["num_errors"] == 1

test/serena/test_dashboard_code.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,9 @@ def __init__(self, root, ls_manager=None):
8181
self._all_tools: dict = {}
8282
self.serena_config = SimpleNamespace(projects=[])
8383

84+
def register_config_changed_callback(self, callback):
85+
pass
86+
8487
def get_active_project(self):
8588
return self._project
8689

@@ -333,6 +336,9 @@ class _AgentNoProject:
333336
_all_tools: dict = {}
334337
serena_config = SimpleNamespace(projects=[])
335338

339+
def register_config_changed_callback(self, callback):
340+
pass
341+
336342
def get_active_project(self):
337343
return None
338344

0 commit comments

Comments
 (0)