Skip to content

Commit 68c4f8d

Browse files
committed
Harden SSH/API network security and bump to 1.0.19
Add shared URL validation helper (pybreeze/utils/network), route AI code review and diagram download through it, disable redirect following on requests.*, switch SSH/SFTP to WarningPolicy with system host keys, and expand CLAUDE.md security checklist.
1 parent 811ab60 commit 68c4f8d

49 files changed

Lines changed: 234 additions & 87 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

CLAUDE.md

Lines changed: 44 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ pybreeze/
3636
├── logging/ # pybreeze_logger
3737
├── file_process/ # File/directory utilities
3838
├── json_format/ # JSON processing
39+
├── network/ # URL validation (SSRF prevention)
3940
└── manager/package_manager/ # PackageManager class
4041
```
4142

@@ -95,25 +96,63 @@ All code must follow secure-by-default principles. Review every change against t
9596
- Never use `subprocess.Popen(..., shell=True)` — always pass argument lists
9697
- Never log or display secrets, tokens, passwords, or API keys
9798
- Use `json.loads()` / `json.dumps()` for serialisation — never pickle
99+
- Never use `yaml.load()` — always use `yaml.safe_load()`
98100
- Validate all user input at system boundaries (file dialogs, URL inputs, network data)
101+
- Handle exceptions without leaking stack traces, file paths, or internal state to the user
99102

100103
### Network requests (SSRF prevention)
101-
- All outbound HTTP requests must go through `diagram_net_utils.safe_download_image()` or equivalent guards
102-
- Only `http://` and `https://` schemes are allowed — block `file://`, `ftp://`, `data:`, `gopher://`
103-
- Resolved IP addresses must be checked against private/loopback/link-local ranges (`ipaddress.is_private`, `is_loopback`, `is_link_local`, `is_reserved`)
104-
- Enforce download size limits (default: 20 MB) and connection timeouts (default: 15s)
105-
- Never pass user-supplied URLs directly to `urlopen()` without validation
104+
- **All** outbound HTTP requests to user-specified URLs must validate the target before connecting:
105+
1. Only `http://` and `https://` schemes — block `file://`, `ftp://`, `data:`, `gopher://`
106+
2. Resolve the hostname and check IPs against private/loopback/link-local/reserved ranges (`ipaddress.is_private`, `is_loopback`, `is_link_local`, `is_reserved`)
107+
3. Enforce connection timeouts (default: 15 s for downloads, 30 s for API calls)
108+
4. Enforce response size limits where applicable (default: 20 MB for binary downloads)
109+
- Reference implementation: `diagram_net_utils._validate_url()` and `safe_download_image()`
110+
- For API-style requests (`requests.get/post`): create or reuse a URL validation helper that performs scheme + IP checks, then call it before every `requests.*` call
111+
- Disable automatic redirect following (`allow_redirects=False`) or re-validate the redirect target to prevent redirect-based SSRF
112+
- Never pass user-supplied URLs directly to `urlopen()` or `requests.*` without validation
113+
114+
### Network requests (TLS / SSH)
115+
- All HTTPS requests must use default TLS verification — never set `verify=False`
116+
- SSH connections: `paramiko.AutoAddPolicy()` accepts any host key and is vulnerable to MITM. Document it as a known limitation in the SSH GUI. Prefer `paramiko.RejectPolicy()` or `paramiko.WarningPolicy()` when non-interactive verification is possible; at minimum, warn the user on first connection to an unknown host
117+
118+
### Subprocess execution
119+
- Always pass argument lists to `subprocess.Popen` / `subprocess.run` — never `shell=True`
120+
- Explicitly set `shell=False` for clarity in new code
121+
- Never interpolate user input into command strings — pass as separate list elements
122+
- Set `timeout` on all `subprocess.run()` calls to prevent hangs
123+
- The IDE intentionally runs user-authored scripts; this is trusted local execution, not arbitrary remote code. Subprocess hardening protects against accidental shell injection, not against malicious local files
124+
125+
### JupyterLab integration
126+
- The embedded JupyterLab server binds to `localhost` only and is intended for local development
127+
- `--ServerApp.token=` and `--ServerApp.password=` are deliberately empty to enable seamless embedding — this is safe only because the server is localhost-only
128+
- Do not change `--ServerApp.ip` to `0.0.0.0` or any externally-reachable address
129+
- `--ServerApp.disable_check_xsrf=True` is required for the embedded QWebEngineView; do not expose the server externally with XSRF disabled
106130

107131
### File I/O
108132
- File read/write paths from user dialogs (`QFileDialog`) are trusted (user-initiated)
109133
- File paths loaded from saved data (`.diagram.json`) must be validated before access:
110134
- Local paths: check `path.is_file()` and verify extension is in an allowlist
111135
- URLs: pass through the same SSRF validation as user-entered URLs
112136
- Never construct file paths by string concatenation with user input — use `pathlib.Path` with validation
137+
- When writing to data directories (`.pybreeze/`), create the directory with `os.makedirs(exist_ok=True)` and always use `encoding="utf-8"`
138+
- Never follow symlinks from untrusted sources — use `Path.resolve(strict=True)` and verify the resolved path is still within expected boundaries
113139

114140
### Qt / UI
115141
- `QGraphicsTextItem` with `TextEditorInteraction` must not be enabled by default — use double-click-to-edit pattern to prevent unintended text selection issues in themed environments
116142
- Plugin loading (`jeditor_plugins/`) uses auto-discovery — only load `.py` files, skip files starting with `_` or `.`
143+
- `QWebEngineView.setUrl()` must only load trusted URLs (localhost or user-confirmed external URLs) — never load untrusted HTML or URLs without user consent
144+
- Never call `QWebEngineView.setHtml()` with unsanitised content — this enables XSS within the embedded browser
145+
146+
### Secrets and credentials
147+
- SSH passwords and private key passphrases are held in memory only during the session — never persist to disk or logs
148+
- Password fields must use `QLineEdit.EchoMode.Password`
149+
- API endpoint URLs may contain embedded tokens — treat URL strings with the same care as credentials (do not log full URLs)
150+
- Environment variables (`PYBREEZE_LOG_MAX_BYTES`, etc.) must never contain secrets; use dedicated secure stores for credentials
151+
152+
### Dependency security
153+
- Pin dependencies to exact versions in `requirements.txt` / `dev_requirements.txt`
154+
- Do not add new dependencies without reviewing their security posture (maintained? known CVEs?)
155+
- Avoid transitive dependency bloat — prefer stdlib solutions when the alternative is a single-function dependency
117156

118157
## Commit & PR rules
119158

pybreeze/__main__.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
from __future__ import annotations
2+
13
from pybreeze import start_editor
24

35
start_editor()

pybreeze/extend/process_executor/python_task_process_manager.py

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
import subprocess
66
import sys
77
import threading
8-
import typing
8+
from typing import Callable
99
from pathlib import Path
1010
from queue import Queue
1111
from threading import Thread
@@ -42,8 +42,8 @@ class TaskProcessManager:
4242
def __init__(
4343
self,
4444
main_window: CodeWindow,
45-
task_done_trigger_function: typing.Callable | None = None,
46-
error_trigger_function: typing.Callable | None = None,
45+
task_done_trigger_function: Callable | None = None,
46+
error_trigger_function: Callable | None = None,
4747
program_buffer_size: int = 1024,
4848
program_encoding: str = "utf-8"
4949
):
@@ -60,8 +60,8 @@ def __init__(
6060
self.run_error_queue: Queue = Queue()
6161
self.process: subprocess.Popen | None = None
6262

63-
self.task_done_trigger_function: typing.Callable = task_done_trigger_function
64-
self.error_trigger_function: typing.Callable = error_trigger_function
63+
self.task_done_trigger_function: Callable = task_done_trigger_function
64+
self.error_trigger_function: Callable = error_trigger_function
6565
self.program_buffer_size = program_buffer_size
6666

6767
def renew_path(self) -> None:

pybreeze/extend/process_executor/test_pioneer/test_pioneer_process_manager.py

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010

1111
from PySide6.QtCore import QTimer
1212
from PySide6.QtGui import QTextCharFormat
13-
from PySide6.QtWidgets import QWidget
1413
from je_editor.pyside_ui.main_ui.save_settings.user_color_setting_file import actually_color_dict
1514
from je_editor.utils.venv_check.check_venv import check_and_choose_venv
1615

@@ -31,7 +30,6 @@ def __init__(
3130
encoding: str = "utf-8",
3231
):
3332
self._main_window: PyBreezeMainWindow = main_window
34-
self._widget: QWidget = main_window.tab_widget.currentWidget()
3533
# Code window init
3634
self._code_window = CodeWindow()
3735
self._main_window.current_run_code_window.append(self._code_window)

pybreeze/extend_multi_language/extend_english.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
from __future__ import annotations
2+
13
from je_editor import english_word_dict
24

35
# PyBreeze-specific English translations

pybreeze/extend_multi_language/extend_traditional_chinese.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
from __future__ import annotations
2+
13
from je_editor import traditional_chinese_word_dict
24

35
# PyBreeze-specific Traditional Chinese translations

pybreeze/extend_multi_language/update_language_dict.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
from __future__ import annotations
2+
13
from pybreeze.extend_multi_language.extend_english import update_english_word_dict
24
from pybreeze.extend_multi_language.extend_traditional_chinese import \
35
update_traditional_chinese_word_dict

pybreeze/pybreeze_ui/connect_gui/ssh/ssh_command_widget.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
from __future__ import annotations
2+
13
import os
24
import re
35

@@ -137,7 +139,11 @@ def connect_ssh(self):
137139

138140
try:
139141
self.ssh_client = paramiko.SSHClient()
140-
self.ssh_client.set_missing_host_key_policy(paramiko.AutoAddPolicy())
142+
self.ssh_client.load_system_host_keys()
143+
self.ssh_client.set_missing_host_key_policy(paramiko.WarningPolicy())
144+
pybreeze_logger.warning(
145+
f"SSH connecting to {host}:{port} — host key will be accepted without verification"
146+
)
141147

142148
if use_key:
143149
if not os.path.exists(key_path):

pybreeze/pybreeze_ui/connect_gui/ssh/ssh_file_viewer_widget.py

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,7 @@
1+
from __future__ import annotations
2+
13
import os
4+
import stat
25
from pathlib import Path
36

47
import paramiko
@@ -10,6 +13,7 @@
1013
from je_editor import language_wrapper
1114

1215
from pybreeze.pybreeze_ui.connect_gui.ssh.ssh_login_widget import LoginWidget
16+
from pybreeze.utils.logging.logger import pybreeze_logger
1317

1418

1519
class SFTPClientWrapper:
@@ -32,7 +36,11 @@ def connect(self, host: str, port: int, username: str, password: str,
3236
"""
3337
self.close()
3438
self._ssh = paramiko.SSHClient()
35-
self._ssh.set_missing_host_key_policy(paramiko.AutoAddPolicy())
39+
self._ssh.load_system_host_keys()
40+
self._ssh.set_missing_host_key_policy(paramiko.WarningPolicy())
41+
pybreeze_logger.warning(
42+
f"SFTP connecting to {host}:{port} — host key will be accepted without verification"
43+
)
3644
if use_key and key_path:
3745
pkey = None
3846
for KeyType in (paramiko.RSAKey, paramiko.Ed25519Key, paramiko.ECDSAKey):
@@ -96,8 +104,6 @@ def is_dir(self, path: str) -> bool:
96104
))
97105
try:
98106
st = self._sftp.stat(path)
99-
# S_ISDIR check via stat.S_ISDIR
100-
import stat
101107
return stat.S_ISDIR(st.st_mode)
102108
except OSError:
103109
return False
@@ -294,7 +300,6 @@ def populate_children(self, parent_item: QTreeWidgetItem):
294300
try:
295301
entries = self.client.list_dir(path)
296302
# Sort: dirs first, then files
297-
import stat
298303
dirs = []
299304
files = []
300305
for e in entries:
@@ -307,10 +312,9 @@ def populate_children(self, parent_item: QTreeWidgetItem):
307312
else:
308313
files.append((name, e))
309314
for name, e in dirs + files:
310-
import stat as _stat
311315
full_path = os.path.join(path if path != "/" else "", name)
312316
full_path = full_path if full_path.startswith("/") else f"/{full_path}"
313-
typ = "dir" if _stat.S_ISDIR(e.st_mode) else "file"
317+
typ = "dir" if stat.S_ISDIR(e.st_mode) else "file"
314318
size = e.st_size if typ == "file" else 0
315319
child = self.make_item(name, typ, size, full_path)
316320
parent_item.addChild(child)

pybreeze/pybreeze_ui/connect_gui/ssh/ssh_login_widget.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
from __future__ import annotations
2+
13
from PySide6.QtWidgets import (
24
QWidget, QHBoxLayout, QVBoxLayout, QLabel,
35
QLineEdit, QSpinBox, QCheckBox, QPushButton

0 commit comments

Comments
 (0)