security: fix path injection CodeQL alerts (#1721)

[mrveiss]

Summary

Fixes all 64 py/path-injection CodeQL alerts from issue #1721.

Approach

• Extended autobot-shared/security/path_validator.py with validate_relative_path() utility
• Applied path validation across 16 backend files to prevent directory traversal attacks

Files Modified (18 files):

• autobot-shared/security/path_validator.py (enhanced with new utility)
• autobot-shared/security/__init__.py (exports)
• autobot-backend/api/files.py (16 fixes)
• autobot-backend/api/data_storage.py (10 fixes)
• autobot-backend/chat_history/session.py (6 fixes)
• autobot-backend/api/filesystem_mcp.py (6 fixes)
• autobot-backend/api/logs.py (5 fixes)
• autobot-backend/api/git_mcp.py (4 fixes)
• autobot-backend/services/terminal_secrets_service.py (3 fixes)
• autobot-backend/api/vnc_manager.py (2 fixes)
• autobot-backend/project_state_tracking/reports.py (2 fixes)
• autobot-backend/api/prompts.py (2 fixes)
• autobot-backend/api/chat_sessions.py (2 fixes)
• autobot-backend/api/chat_knowledge.py (1 fix)
• autobot-backend/api/codebase_analytics/endpoints/ownership.py (1 fix)
• autobot-backend/api/codebase_analytics/endpoints/environment.py (1 fix)
• autobot-backend/services/fast_document_scanner.py (1 fix)
• autobot-backend/secure_sandbox_executor.py (1 fix)

Part of #1721

Test plan

• CodeQL re-scan shows path-injection alerts resolved
• No new alerts introduced
• File operations still work correctly with valid paths
• Path traversal attempts are properly rejected
• Shared utility has correct behavior for edge cases

[github-actions]

✅ SSOT Configuration Compliance: Passing

🎉 No hardcoded values detected that have SSOT config equivalents!

[mrveiss]

Code Review: security: fix path injection CodeQL alerts (#1721)

Good approach centralizing path validation via validate_relative_path(). However, several issues must be addressed before merge.

CRITICAL: Incomplete coverage in filesystem_mcp.py

6+ endpoints still use raw request.path after is_path_allowed() check — the same vulnerability class this PR aims to fix:

• _read_single_file_for_batch() (line ~804)
• create_directory_mcp() (line ~1057) — calls os.makedirs(request.path)
• list_directory_mcp() (line ~1089) — calls os.listdir(request.path)
• move_file_mcp() (line ~1267) — calls shutil.move(request.source, request.destination)
• search_files_mcp() (line ~1319) — calls os.walk(request.path)
• directory_tree_mcp() (line ~1378)
• get_file_info_mcp() (line ~1441) — calls os.stat(request.path)

These all need conversion to _validated_path(). CodeQL will continue flagging these.

HIGH: validate_path() exception handler leaks internal paths

When must_exist=True and path is outside all roots but doesn't exist on disk, the relative_to() ValueError message leaks resolved paths. Restructure to separate the two ValueError sources.

HIGH: reports.py uses Path(".") as base_dir

CWD-dependent — could be / depending on deployment. Use explicit absolute base path.

MEDIUM: No unit tests for security utility

validate_relative_path() is security-critical and needs dedicated tests: traversal, null bytes, empty string, absolute path, symlink escape, must_exist variants.

LOW findings

• Import ordering regression in secure_sandbox_executor.py
• Redundant manual traversal check in chat_sessions.py
• Inconsistent inline vs module-level imports
• TOCTOU symlink window (accepted risk — document in docstring)

Verdict: Request changes. Findings 1, 2, 3 are blocking.

[mrveiss]

Re-Review: path injection fixes after feedback (#1721)

All 3 critical/high findings from the original review are resolved:

1. filesystem_mcp.py coverage — All 7 endpoints now use _validated_path(). No remaining is_path_allowed() call sites in any endpoint. Fixed.

2. validate_path() exception handler — relative_to() failure and must_exist check are now properly separated. No path leakage. Fixed.

3. reports.py Path(".") — else branch correctly uses PROJECT_ROOT. Fixed.

Minor remaining nit

The if branch in reports.py still uses relative data_base = Path("data/reports/state_tracking") — should be PROJECT_ROOT / "data/reports/state_tracking" for consistency. Low risk (backend CWD is controlled), trivial fix.

Verdict: Approve (with minor nit). The blocking security issues are resolved.

[mrveiss]

Code review

No issues found. Checked for bugs and CLAUDE.md compliance.

Note: An earlier version of this PR had validate_relative_path(output_path, Path(".")) in reports.py using CWD as the trust anchor, but this was correctly fixed in commit 38390c9 to use PROJECT_ROOT before merge.

Generated with Claude Code