Skip to content

Commit 45d345b

Browse files
authored
Merge pull request #121 from Integration-Automation/dev
Add SonarQube/Codacy conventions and harden network security
2 parents 16ee980 + ccb7356 commit 45d345b

50 files changed

Lines changed: 436 additions & 91 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: 116 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,135 @@ 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: never use `paramiko.AutoAddPolicy()` or `paramiko.WarningPolicy()` — both silently accept unknown host keys and are vulnerable to MITM. Use `InteractiveHostKeyPolicy` from `pybreeze.pybreeze_ui.connect_gui.ssh.ssh_host_key_policy` (via `apply_host_key_policy(client, parent_widget)`), which prompts the user with the SHA256 fingerprint on first connection and persists confirmed keys to `~/.pybreeze/ssh_known_hosts`
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
156+
157+
## Code quality (SonarQube / Codacy compliance)
158+
159+
All code must satisfy common static-analysis rules enforced by SonarQube and Codacy. Review each change against the checklist below.
160+
161+
### Complexity & size
162+
- Cyclomatic complexity per function: ≤ 15 (hard cap 20). Break large branches into helpers
163+
- Cognitive complexity per function: ≤ 15. Flatten nested `if`/`for`/`try` chains with early returns or guard clauses
164+
- Function length: ≤ 75 lines of code (excluding docstring / blank lines). Extract helpers past that
165+
- Parameter count: ≤ 7 per function/method. Use a dataclass or typed dict when more are needed
166+
- Nesting depth: ≤ 4 levels of `if`/`for`/`while`/`try`. Refactor with early returns instead of pyramids
167+
- File length: ≤ 1000 lines — split modules past that
168+
- Class `__init__`: keep attribute count reasonable; if a class has > 15 instance attributes, split responsibilities
169+
170+
### Exception handling
171+
- Never use bare `except:` — always specify exception types
172+
- Avoid catching `Exception` or `BaseException` unless immediately re-raising or logging and re-raising with context
173+
- Never `pass` silently inside `except` — log the error via `pybreeze_logger` (at minimum `.debug()`) with context
174+
- Do not `return` / `break` / `continue` inside a `finally` block — it swallows exceptions
175+
- Custom exceptions must inherit from `ITEException`; never `raise Exception(...)` directly
176+
- Use `raise ... from err` (or `raise ... from None`) when re-raising to preserve / suppress the chain explicitly
177+
178+
### Pythonic correctness
179+
- Compare with `None` using `is` / `is not`, never `==` / `!=`
180+
- Type checks use `isinstance(obj, T)`, never `type(obj) == T`
181+
- Never use mutable default arguments (`def f(x=[])`) — use `None` and initialise inside
182+
- Prefer f-strings over `%` formatting or `str.format()`
183+
- Use context managers (`with open(...) as f:`) for every file / socket / lock — never leave resources to GC
184+
- Use `enumerate()` instead of `range(len(...))` when the index is needed alongside the item
185+
- Use `dict.get(key, default)` instead of `key in dict and dict[key]` patterns
186+
- Use set / dict comprehensions when clearer than manual loops; avoid comprehensions with side effects
187+
188+
### Naming & style (PEP 8)
189+
- `snake_case` for functions, methods, variables, module names
190+
- `PascalCase` for classes
191+
- `UPPER_SNAKE_CASE` for module-level constants
192+
- `_leading_underscore` for protected / internal members; never use `__dunder__` for custom attributes
193+
- No single-letter names except loop indices (`i`, `j`) or conventional math (`x`, `y`)
194+
- Do not shadow built-ins (`id`, `type`, `list`, `dict`, `input`, `file`, `open`, etc.) — rename the local variable
195+
196+
### Duplication & dead code
197+
- String literal used 3+ times in the same module → extract a module-level constant
198+
- Identical 6+ line blocks in 2+ places → extract a helper function
199+
- Remove unused imports, unused parameters, unused local variables, unreachable code after `return` / `raise`
200+
- No commented-out code blocks — delete them (git history is the archive)
201+
- No `TODO` / `FIXME` / `XXX` without an accompanying issue reference (`# TODO(#123): ...`)
202+
203+
### Logging, printing, assertions
204+
- Never use `print()` for diagnostics in library / runtime code — use `pybreeze_logger`
205+
- Use lazy logging (`logger.debug("x=%s", x)`) — avoid eager f-string formatting inside log calls on hot paths
206+
- Never use `assert` for runtime validation (Python strips assertions with `-O`). Use explicit `if … raise …` instead; `assert` is only for test code
207+
208+
### Hardcoded values & secrets
209+
- No hardcoded passwords, tokens, API keys, or secrets — use env vars or a config file excluded from VCS
210+
- No hardcoded IP addresses or hostnames outside of `localhost` / documented loopback — use config
211+
- Magic numbers (except 0, 1, -1) should be named constants when repeated or non-obvious
212+
213+
### Boolean & return hygiene
214+
- Replace `if cond: return True else: return False` with `return bool(cond)` or `return cond`
215+
- Replace `if x == True` / `if x == False` with `if x` / `if not x`
216+
- A function should have a consistent return type — never mix `return value` and bare `return` (returns `None`) on meaningful paths unless explicitly documented
217+
- Do not return inside a generator function (`yield` + `return value` is a syntax pitfall)
218+
219+
### Imports
220+
- One import per line for `import` statements; grouped `from x import a, b` is fine
221+
- Order: stdlib → third-party → first-party (`pybreeze.*`) — separated by blank lines
222+
- No wildcard imports (`from x import *`) outside of `__init__.py` re-exports
223+
- No relative imports beyond one level (`from ..pkg import x` OK, `from ...pkg import x` avoid)
224+
225+
### Running the linters
226+
- Before committing any non-trivial change, run `ruff check pybreeze/` locally to catch these rules — `ruff` covers the majority of SonarQube/Codacy Python rules
227+
- When adding a new rule exception, justify it in a `# noqa: RULE` comment with a short reason — never blanket-disable
117228

118229
## Commit & PR rules
119230

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: 9 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
@@ -99,6 +101,13 @@
99101
"test_pioneer_create_template_label": "Create TestPioneer Yaml template",
100102
"test_pioneer_run_yaml": "Execute Test Pioneer Yaml",
101103
"test_pioneer_not_choose_yaml": "Please choose a Yaml file",
104+
# SSH host key verification
105+
"ssh_host_key_policy_dialog_title_verify_host": "Verify SSH host key",
106+
"ssh_host_key_policy_dialog_message_verify_host": (
107+
"The authenticity of host '{host}' cannot be established.\n"
108+
"{key_type} key fingerprint is {fingerprint}.\n\n"
109+
"Do you want to trust this host and continue connecting?"
110+
),
102111
# SSH command widget
103112
"ssh_command_widget_window_title_ssh_command_widget": "SSH Command Widget",
104113
"ssh_command_widget_button_label_send_command": "Send",

pybreeze/extend_multi_language/extend_traditional_chinese.py

Lines changed: 9 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
@@ -99,6 +101,13 @@
99101
"test_pioneer_create_template_label": "建立 TestPioneer Yaml 模板",
100102
"test_pioneer_run_yaml": "執行 Test Pioneer Yaml",
101103
"test_pioneer_not_choose_yaml": "請選擇 Yaml 檔案",
104+
# SSH host key verification
105+
"ssh_host_key_policy_dialog_title_verify_host": "驗證 SSH 主機金鑰",
106+
"ssh_host_key_policy_dialog_message_verify_host": (
107+
"無法驗證主機 '{host}' 的真實性。\n"
108+
"{key_type} 金鑰指紋為 {fingerprint}。\n\n"
109+
"是否信任此主機並繼續連線?"
110+
),
102111
# SSH command widget
103112
"ssh_command_widget_window_title_ssh_command_widget": "SSH 指令介面",
104113
"ssh_command_widget_button_label_send_command": "送出",

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: 5 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

@@ -10,6 +12,7 @@
1012
)
1113
from je_editor import language_wrapper
1214

15+
from pybreeze.pybreeze_ui.connect_gui.ssh.ssh_host_key_policy import apply_host_key_policy
1316
from pybreeze.pybreeze_ui.connect_gui.ssh.ssh_login_widget import LoginWidget
1417
from pybreeze.utils.logging.logger import pybreeze_logger
1518

@@ -137,7 +140,8 @@ def connect_ssh(self):
137140

138141
try:
139142
self.ssh_client = paramiko.SSHClient()
140-
self.ssh_client.set_missing_host_key_policy(paramiko.AutoAddPolicy())
143+
apply_host_key_policy(self.ssh_client, self)
144+
pybreeze_logger.info("SSH connecting to %s:%s", host, port)
141145

142146
if use_key:
143147
if not os.path.exists(key_path):

pybreeze/pybreeze_ui/connect_gui/ssh/ssh_file_viewer_widget.py

Lines changed: 11 additions & 8 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
@@ -9,7 +12,9 @@
912
)
1013
from je_editor import language_wrapper
1114

15+
from pybreeze.pybreeze_ui.connect_gui.ssh.ssh_host_key_policy import apply_host_key_policy
1216
from pybreeze.pybreeze_ui.connect_gui.ssh.ssh_login_widget import LoginWidget
17+
from pybreeze.utils.logging.logger import pybreeze_logger
1318

1419

1520
class SFTPClientWrapper:
@@ -25,14 +30,16 @@ def __init__(self):
2530
self.root_path: str = "/"
2631

2732
def connect(self, host: str, port: int, username: str, password: str,
28-
use_key: bool = False, key_path: str = ""):
33+
use_key: bool = False, key_path: str = "",
34+
parent_widget: QWidget | None = None):
2935
"""
3036
Establish SSH + SFTP connection.
3137
建立 SSH + SFTP 連線。
3238
"""
3339
self.close()
3440
self._ssh = paramiko.SSHClient()
35-
self._ssh.set_missing_host_key_policy(paramiko.AutoAddPolicy())
41+
apply_host_key_policy(self._ssh, parent_widget)
42+
pybreeze_logger.info("SFTP connecting to %s:%s", host, port)
3643
if use_key and key_path:
3744
pkey = None
3845
for KeyType in (paramiko.RSAKey, paramiko.Ed25519Key, paramiko.ECDSAKey):
@@ -96,8 +103,6 @@ def is_dir(self, path: str) -> bool:
96103
))
97104
try:
98105
st = self._sftp.stat(path)
99-
# S_ISDIR check via stat.S_ISDIR
100-
import stat
101106
return stat.S_ISDIR(st.st_mode)
102107
except OSError:
103108
return False
@@ -210,7 +215,7 @@ def _connect(self):
210215
self.word_dict.get("ssh_file_viewer_dialog_message_missing_input"))
211216
return
212217
try:
213-
self.client.connect(host, port, user, pwd, use_key, key_path)
218+
self.client.connect(host, port, user, pwd, use_key, key_path, parent_widget=self)
214219
self.load_root("/")
215220
except Exception as e:
216221
QMessageBox.critical(
@@ -294,7 +299,6 @@ def populate_children(self, parent_item: QTreeWidgetItem):
294299
try:
295300
entries = self.client.list_dir(path)
296301
# Sort: dirs first, then files
297-
import stat
298302
dirs = []
299303
files = []
300304
for e in entries:
@@ -307,10 +311,9 @@ def populate_children(self, parent_item: QTreeWidgetItem):
307311
else:
308312
files.append((name, e))
309313
for name, e in dirs + files:
310-
import stat as _stat
311314
full_path = os.path.join(path if path != "/" else "", name)
312315
full_path = full_path if full_path.startswith("/") else f"/{full_path}"
313-
typ = "dir" if _stat.S_ISDIR(e.st_mode) else "file"
316+
typ = "dir" if stat.S_ISDIR(e.st_mode) else "file"
314317
size = e.st_size if typ == "file" else 0
315318
child = self.make_item(name, typ, size, full_path)
316319
parent_item.addChild(child)

0 commit comments

Comments
 (0)