Vulnerability: The src/nodetool/api/file.py module provided upload and download endpoints that allowed arbitrary file system access (read/write) without directory restrictions. While this module is conditionally loaded (supposedly only in non-production), it lacked internal safeguards, making it a high-risk vector if accidentally exposed or enabled.
Learning: src/nodetool/api/workspace.py correctly implemented ensure_within_root, but file.py bypassed this pattern entirely, likely assuming "desktop mode" implies trusted local user. Security boundaries were inconsistent across modules handling file operations.
Prevention: Always enforce path restrictions (allowlisting) at the API handler level, regardless of environment flags. Do not rely solely on Environment.is_production() to gate dangerous functionality.
Vulnerability: The _is_safe_path function effectively acted as a blacklist rather than a whitelist because it returned True as a default instead of is_in_safe_root. This combined with missing checks in /api/files/list and /api/files/info meant any user could perform arbitrary directory listing and access files outside of safe paths as long as they were not explicitly explicitly blacklisted in SENSITIVE_PATHS.
Learning: Returning True by default in validation functions designed around whitelists turns the whitelist into a blacklist. Furthermore, when creating helper functions for security, they must consistently be called by all related endpoints; leaving out _is_safe_path in listing endpoints exposes the directory structure.
Prevention: Ensure validation functions fail closed (e.g. return is_in_safe_root instead of return True) and double-check that all REST endpoints mapped to similar underlying resources correctly share the security guards.
Vulnerability: The _legacy_export_to_video_bytes function in src/nodetool/media/video/video_utils.py used a hardcoded path (/tmp/temp_video.mp4) for temporary file storage during video encoding. This allowed for symlink attacks, race conditions, and cross-tenant data leakage if multiple processes or users executed the function concurrently.
Learning: Hardcoded temporary paths are a common anti-pattern that can easily be overlooked in fallback or legacy code paths. They violate the principle of isolation and can lead to severe vulnerabilities in multi-user environments.
Prevention: Always use secure temporary file creation mechanisms like tempfile.NamedTemporaryFile with appropriate cleanup logic (e.g., delete=False followed by a try...finally block for manual removal) to guarantee uniqueness and prevent race conditions.
Vulnerability: SQL Injection in Data Definition Language (DDL) queries. Methods like create_table, migrate_table, and create_index in src/nodetool/models/postgres_adapter.py used unsafe Python f-string concatenation for table names, column names, and types.
Learning: These methods construct structural modifications and typically cannot use normal parameterization ($1 or %s) provided by DB drivers for values. As a result, developers sometimes mistakenly fall back to standard string formatting.
Prevention: Always use safe query composition objects specifically designed for DDL. In psycopg (PostgreSQL), strictly use psycopg.sql.SQL and psycopg.sql.Identifier to safely construct dynamic table, column, and schema names in database adapters.