Fix Multi AI Audit consensus findings (12 issues)

.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 <package>`
+- `pip install` や素の `python` コマンドは使わない

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コメントバイパスを修正（セキュリティ）

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 <dev@example.com>
-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 <<EOF_EXEC > "$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"
 

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 "$@"

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()

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

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:

src/sqliteviewer/resources/__init__.py

@@ -3,17 +3,14 @@
 from __future__ import annotations
 
 from importlib import resources
-from pathlib import Path
-from typing import Optional
 
 from PyQt6.QtGui import QIcon
 
 
 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:

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)

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()