diff --git a/.claude/rules/code-quality.md b/.claude/rules/code-quality.md new file mode 100644 index 0000000..a829e44 --- /dev/null +++ b/.claude/rules/code-quality.md @@ -0,0 +1,37 @@ +# Code Quality Rules + +## 未使用 import を残さない +- import 文を追加・変更する際は、ファイル内で実際に使用されているか確認する +- `from X import a, b` で `b` を使わないなら `from X import a` にする +- 既存コードを変更して不要になった import も同時に削除する + +## セキュリティチェックは初回から網羅的に実装する +- SQL のサニタイズ・検証ロジックを書く際は、コメント(`--`, `/* */`)と文字列リテラル(`'...'`)とダブルクォート識別子(`"..."`)をすべて考慮する +- 「まずコメントだけ対応して後でリテラルも」ではなく、初回実装ですべてのクォート形式をカバーする +- 文字列マッチングでキーワード検出する場合、ノイズ除去 → キーワード検出の順序を徹底する + +## スクリプトのランチャーは `python3 -m` を使う +- Python パッケージの起動は `python3 path/to/__main__.py` ではなく `python3 -m module_name` を使う +- `__main__.py` を直接実行すると相対インポートが壊れる +- `PYTHONPATH` を適切に設定した上で `-m` で起動する +- 開発時は `uv run python -m sqliteviewer` を使う + +## バージョン・定数の二重管理を避ける +- バージョン番号は `pyproject.toml` を Single Source of Truth とする +- シェルスクリプト等で必要な場合は `pyproject.toml` から動的に取得する(`grep`/`sed` を使い、Python の `tomllib` 等に依存しない) +- `requires-python` の最低バージョンで動作するか常に確認する(例: `tomllib` は 3.11+、`pyproject.toml` は `>=3.10`) +- ハードコードした定数が `pyproject.toml` と乖離していないか確認する + +## 変更の波及先を確認する +- パッケージ名・バージョン等のプロジェクト全体に影響する変更時は、以下をすべて確認する: + - `pyproject.toml` + - `scripts/build_deb.sh`(ランチャー、DEBIAN/control、wheel ファイル名) + - `scripts/run_app.sh` +- 一箇所だけ変更して他を放置しない + +## Python 環境は uv を使う +- パッケージ管理・仮想環境・コマンド実行はすべて `uv` を使う +- テスト実行: `uv run python -m pytest tests/ -v` +- アプリ起動: `uv run python -m sqliteviewer` +- 依存追加: `uv add ` +- `pip install` や素の `python` コマンドは使わない diff --git a/PLAN.md b/PLAN.md index 43e6db6..341ec7e 100644 --- a/PLAN.md +++ b/PLAN.md @@ -17,8 +17,9 @@ SQLiteViewは現在、読み取り専用のSQLiteビューア(SELECT/WITH/PRAG ### Step 1: database.py — DB層の変更(完了) - [x] `QueryResult` に `affected_rows: Optional[int]` と `is_write_operation: bool` を追加 -- [x] `_classify_query()` を追加 — `read`/`dml`/`ddl`/`tcl`/`unknown` を返す +- [x] `classify_query()` を追加 — `read`/`dml`/`ddl`/`tcl`/`unknown` を返す(公開メソッド) - [x] `is_destructive_query()` を追加 — DROP・WHERE無しDELETEを検出し `(bool, reason)` を返す +- [x] `_strip_sql_comments()` を追加 — `--` / `/* */` コメントを除去(WHERE句バイパス防止) - [x] `execute_query()` を書き換え — `_assert_read_only()` 削除、`cursor.description is None` で書き込み判定 - [x] `_assert_read_only()` を削除 - [x] docstring `"read-only SQLite interactions"` → `"SQLite database interactions"` に更新 @@ -32,7 +33,7 @@ SQLiteViewは現在、読み取り専用のSQLiteビューア(SELECT/WITH/PRAG - [x] DELETE のテストを追加 - [x] DDL(CREATE TABLE / DROP TABLE)のテストを追加 - [x] トランザクション(BEGIN→INSERT→ROLLBACK)のテストを追加 -- [x] クエリ分類(`_classify_query`)のテストを追加 +- [x] クエリ分類(`classify_query`)のテストを追加 - [x] 破壊的操作検出(`is_destructive_query`)のテストを追加 > 10テストすべて pass 確認済み。 @@ -51,6 +52,7 @@ SQLiteViewは現在、読み取り専用のSQLiteビューア(SELECT/WITH/PRAG - [x] `_run_query()` を書き換え — 破壊的クエリの確認ダイアログ(`QMessageBox.warning`)追加 - [x] 書き込み結果の表示: `"{N} row(s) affected"` または `"Statement executed successfully"` - [x] `_refresh_after_write()` を追加 — DDL後はテーブルリスト・プレビュー・スキーマをリフレッシュ、DML後は選択中テーブルのプレビューをリフレッシュ +- [x] write操作後に `self.query_result = None` をリセット — 古いSELECT結果が誤ってExportされる問題を修正 - [x] `_refresh_tables()` を改善 — リフレッシュ後に以前の選択テーブルを復元 - [x] About ダイアログのバージョンを `0.1.0` → `0.2.1` に修正 @@ -67,5 +69,9 @@ uv run sqliteview test.db ## 現状 -**実装完了。** 全ステップの実装・テストが完了しており、10テストすべて pass。 +**実装完了・マージ済み。** 全ステップの実装・テストが完了しており、10テストすべて pass。 SQLiteView は INSERT/UPDATE/DELETE/CREATE/DROP/BEGIN/COMMIT/ROLLBACK 等の書き込みクエリを実行可能な SQL クライアントとなった。 + +PR #3 にてレビュー指摘対応・CIエラー修正を行い、main ブランチへのマージ完了(2026-02-27)。 +- `scripts/build_deb.sh` のパッケージ名・バージョンを修正(CIエラー解消) +- WHERE句チェックのSQLコメントバイパスを修正(セキュリティ) diff --git a/scripts/build_deb.sh b/scripts/build_deb.sh index 5bcf065..41e2980 100755 --- a/scripts/build_deb.sh +++ b/scripts/build_deb.sh @@ -5,7 +5,7 @@ ROOT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" DIST_DIR="$ROOT_DIR/dist" BUILD_DIR="$DIST_DIR/deb_build" PACKAGE="sqliteview" -VERSION="0.2.1" +VERSION=$(grep -m1 '^version' "$ROOT_DIR/pyproject.toml" | sed 's/.*"\(.*\)"/\1/') ARCH="all" rm -rf "$BUILD_DIR" @@ -45,7 +45,7 @@ Section: utils Priority: optional Architecture: all Maintainer: SQLite Viewer Team -Depends: python3 (>= 3.10) +Depends: python3 (>= 3.10), python3-pyqt6 Description: PyQt6-based SQLite database client for Ubuntu. SQLiteView provides a desktop UI for browsing tables, running ad-hoc queries, and exporting results. Packaged with its Python @@ -55,8 +55,8 @@ EOF_CONTROL cat < "$BUILD_DIR/usr/bin/$PACKAGE" #!/usr/bin/env bash set -euo pipefail -SCRIPT_DIR="/usr/lib/$PACKAGE" -exec python3 "\$SCRIPT_DIR/sqliteviewer/__main__.py" "\$@" +export PYTHONPATH="/usr/lib/$PACKAGE" +exec python3 -m sqliteviewer "\$@" EOF_EXEC chmod +x "$BUILD_DIR/usr/bin/$PACKAGE" diff --git a/scripts/run_app.sh b/scripts/run_app.sh index 7bbb395..85a5e1f 100755 --- a/scripts/run_app.sh +++ b/scripts/run_app.sh @@ -1,4 +1,5 @@ #!/usr/bin/env bash set -euo pipefail ROOT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" -exec python3 "$ROOT_DIR/src/sqliteviewer/__main__.py" "$@" +export PYTHONPATH="$ROOT_DIR/src" +exec python3 -m sqliteviewer "$@" diff --git a/src/sqliteviewer/app.py b/src/sqliteviewer/app.py index 2d24f90..3a1fa09 100644 --- a/src/sqliteviewer/app.py +++ b/src/sqliteviewer/app.py @@ -20,7 +20,6 @@ def run(initial_path: Optional[str] = None) -> int: app = QApplication(sys.argv) app.setApplicationName("SQLite View") app.setOrganizationName("SQLiteView") - app.setOrganizationDomain("example.com") owns_app = True window = MainWindow() diff --git a/src/sqliteviewer/database.py b/src/sqliteviewer/database.py index 92b0a1b..2c85147 100644 --- a/src/sqliteviewer/database.py +++ b/src/sqliteviewer/database.py @@ -2,7 +2,7 @@ from __future__ import annotations -from dataclasses import dataclass, field +from dataclasses import dataclass from pathlib import Path from typing import Iterable, List, Optional, Sequence, Tuple @@ -170,13 +170,13 @@ def is_destructive_query(self, sql: str) -> Tuple[bool, str]: if keyword == "DROP": return True, "This will permanently drop the object." if keyword == "DELETE": - stripped = self._strip_sql_comments(sql).upper() + stripped = self._strip_sql_noise(sql).upper() if "WHERE" not in stripped: return True, "DELETE without WHERE will remove all rows." return False, "" - def _strip_sql_comments(self, sql: str) -> str: - """Remove SQL comments (-- and /* */) from a statement.""" + def _strip_sql_noise(self, sql: str) -> str: + """Remove SQL comments and string literals from a statement.""" result = [] i = 0 @@ -188,16 +188,14 @@ def _strip_sql_comments(self, sql: str) -> str: elif sql[i] == '/' and i + 1 < length and sql[i + 1] == '*': end = sql.find('*/', i + 2) i = length if end == -1 else end + 2 - elif sql[i] == "'": - result.append(sql[i]) + elif sql[i] in ("'", '"'): + quote = sql[i] i += 1 while i < length: - result.append(sql[i]) - if sql[i] == "'" and (i + 1 >= length or sql[i + 1] != "'"): + if sql[i] == quote and (i + 1 >= length or sql[i + 1] != quote): i += 1 break - if sql[i] == "'" and i + 1 < length and sql[i + 1] == "'": - result.append(sql[i + 1]) + if sql[i] == quote and i + 1 < length and sql[i + 1] == quote: i += 2 else: i += 1 diff --git a/src/sqliteviewer/mainwindow.py b/src/sqliteviewer/mainwindow.py index e9f8c89..1fbe070 100644 --- a/src/sqliteviewer/mainwindow.py +++ b/src/sqliteviewer/mainwindow.py @@ -312,12 +312,10 @@ def _refresh_after_write(self, sql: str) -> None: self._load_table_preview(table_name) self._load_table_schema(table_name) elif query_type == "dml": - # Refresh preview only if the affected table is currently selected selected_items = self.table_list.selectedItems() if selected_items: table_name = selected_items[0].text() - if table_name.lower() in sql.lower(): - self._load_table_preview(table_name) + self._load_table_preview(table_name) def _export_results(self) -> None: if not self.query_result or not self.query_result.columns: diff --git a/src/sqliteviewer/resources/__init__.py b/src/sqliteviewer/resources/__init__.py index cd6fcb8..4e08b61 100644 --- a/src/sqliteviewer/resources/__init__.py +++ b/src/sqliteviewer/resources/__init__.py @@ -3,8 +3,6 @@ from __future__ import annotations from importlib import resources -from pathlib import Path -from typing import Optional from PyQt6.QtGui import QIcon @@ -12,8 +10,7 @@ def resource_path(name: str) -> str: """Return the filesystem path to a resource bundled with the package.""" - with resources.as_file(resources.files(__package__).joinpath(name)) as path: - return str(path) + return str(resources.files(__package__).joinpath(name)) def load_icon() -> QIcon: diff --git a/src/sqliteviewer/sql_highlighter.py b/src/sqliteviewer/sql_highlighter.py index 5327d32..bd4e85b 100644 --- a/src/sqliteviewer/sql_highlighter.py +++ b/src/sqliteviewer/sql_highlighter.py @@ -126,15 +126,12 @@ def __init__(self, document) -> None: self.string_expression = QRegularExpression(r"'([^']|'')*'") self.number_expression = QRegularExpression(r"\b\d+(\.\d+)?\b") - self.keyword_patterns = [] - for keyword in self.KEYWORDS: - pattern = QRegularExpression(rf"\b{keyword}\b") - pattern.setPatternOptions(QRegularExpression.PatternOption.CaseInsensitiveOption) - self.keyword_patterns.append(pattern) + pattern_str = r"\b(" + "|".join(self.KEYWORDS) + r")\b" + self.keyword_pattern = QRegularExpression(pattern_str) + self.keyword_pattern.setPatternOptions(QRegularExpression.PatternOption.CaseInsensitiveOption) def highlightBlock(self, text: str) -> None: # noqa: N802 (Qt API signature) - for pattern in self.keyword_patterns: - self._apply_regex(pattern, text, self.keyword_format) + self._apply_regex(self.keyword_pattern, text, self.keyword_format) self._apply_regex(self.comment_expression, text, self.comment_format) self._apply_regex(self.string_expression, text, self.string_format) self._apply_regex(self.number_expression, text, self.number_format) diff --git a/tests/test_database.py b/tests/test_database.py index 06e2cc8..8059d7e 100644 --- a/tests/test_database.py +++ b/tests/test_database.py @@ -127,6 +127,52 @@ def test_schema_contains_create_statement(self) -> None: self.assertIn("CREATE TABLE", schema) self.assertIn("users", schema) + def test_execute_query_empty_raises(self) -> None: + with self.assertRaises(DatabaseError): + self.service.execute_query("") + + def test_execute_query_invalid_sql_raises(self) -> None: + with self.assertRaises(DatabaseError): + self.service.execute_query("NOT VALID SQL !!!") + + def test_operations_without_open_raises(self) -> None: + svc = DatabaseService() + with self.assertRaises(DatabaseError): + svc.list_tables() + with self.assertRaises(DatabaseError): + svc.get_table_preview("users") + with self.assertRaises(DatabaseError): + svc.execute_query("SELECT 1") + + def test_open_nonexistent_file_raises(self) -> None: + svc = DatabaseService() + with self.assertRaises(DatabaseError): + svc.open("/tmp/nonexistent_db_file_12345.db") + + def test_is_destructive_with_where_in_string_literal(self) -> None: + sql = "DELETE FROM users WHERE name = 'WHERE'" + is_d, _ = self.service.is_destructive_query(sql) + self.assertFalse(is_d) + + sql_no_where = "DELETE FROM users -- WHERE\n" + is_d, _ = self.service.is_destructive_query(sql_no_where) + self.assertTrue(is_d) + + sql_literal_only = "DELETE FROM users WHERE name = 'test'" + is_d, _ = self.service.is_destructive_query(sql_literal_only) + self.assertFalse(is_d) + + def test_is_destructive_with_where_in_double_quoted_identifier(self) -> None: + # "WHERE" as a column name in double-quoted identifier should be stripped + sql = 'DELETE FROM users WHERE "WHERE" = 1' + is_d, _ = self.service.is_destructive_query(sql) + self.assertFalse(is_d) + + # WHERE only inside double-quoted identifier — no real WHERE clause + sql_no_where = 'DELETE FROM "WHERE"' + is_d, _ = self.service.is_destructive_query(sql_no_where) + self.assertTrue(is_d) + if __name__ == "__main__": unittest.main()