From dcf79844ab748d85be030de1865212f2dd282f95 Mon Sep 17 00:00:00 2001 From: TTMK7777 Date: Fri, 31 Jul 2026 10:57:02 +0900 Subject: [PATCH] =?UTF-8?q?SEC007=20(=E3=82=BB=E3=82=AD=E3=83=A5=E3=83=AA?= =?UTF-8?q?=E3=83=86=E3=82=A3=E3=83=98=E3=83=83=E3=83=80=E3=83=BC=E6=AC=A0?= =?UTF-8?q?=E8=90=BD)=20=E3=82=92=E5=89=8A=E9=99=A4?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 行単位・ファイル単位のスキャナでは原理的に判定できないため、ルールごと削除する。 理由: セキュリティヘッダーは専用のミドルウェアモジュールに置き、アプリ生成箇所で add_middleware する構成が標準であり、ヘッダー名は定義側のファイルにしか 現れない。実コーパス計測では、正しくミドルウェアを実装しているアプリを 「未設定」と誤って指摘した。 ORICON-Chart-Dashboard/web/app.py を指摘 -> 実際は web/middleware.py の SecurityHeadersMiddleware で設定済み app.py:244 で add_middleware(SecurityHeadersMiddleware) 済み 誤検知を絞り込む方向では解決できず、必要なのはモジュール横断の解析。 それはこのツールの設計範囲外。同種の検査が必要な場合は Semgrep / CodeQL を使う。 削除内容: - validator.py _scan_security_headers / FASTAPI_APP_RE / REQUIRED_SECURITY_HEADERS と scan_file からの呼び出し - config security_rules.check_security_headers（残っていても無視される） - tests/test_security_headers.py（7 テスト） - README / spec.md / QUICKSTART / docs/dev/architecture.md / llms.txt x2 / plan.md / todo.md の記述 ルール数 21 -> 20、テスト 160 -> 153。 あわせて CHANGELOG に「他リポジトリへの展開を見送った」記録を残した。 実コーパス計測で指摘は 121 -> 16 件まで下がったが、残り 16 件を全件目視した 結果、行動を要するものは 0〜2 件だった。主因は精度ではなく、対象コーパスに 当該ルールが狙う脆弱性が残っていないこと。 検証: - pytest 153 件パス - 同一コーパスで再計測し SEC007 が 0 件になったことを確認。 他ルールの件数は削除前と一致 (SEC006 9 / SEC011 1 / SEC013 4) --- CHANGELOG.md | 30 ++++++++++++ QUICKSTART.md | 1 - README.md | 8 +-- config/validator_config.json | 1 - docs/dev/architecture.md | 2 +- docs/llms.txt | 1 - llms.txt | 1 - plan.md | 2 +- spec.md | 3 +- tests/test_config.py | 1 - tests/test_security_headers.py | 90 ---------------------------------- tests/test_suppressions.py | 4 +- validator.py | 54 +++++--------------- 13 files changed, 51 insertions(+), 147 deletions(-) delete mode 100644 tests/test_security_headers.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 9feeacd..43cbd63 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,36 @@ ## [Unreleased] +### Removed +- **SEC007（FastAPI のセキュリティヘッダー欠落）を削除**。行単位・ファイル単位の + スキャナでは原理的に判定できないため。セキュリティヘッダーは専用のミドルウェア + モジュールに置き、アプリ生成箇所で `add_middleware` する構成が標準であり、 + ヘッダー名は定義側のファイルにしか現れない。実コーパス計測では、正しく + ミドルウェアを実装しているアプリを「未設定」と誤指摘した。同種の検査が必要な + 場合は Semgrep / CodeQL を使うこと。 +- `security_rules.check_security_headers` 設定キーを廃止（残っていても無視される）。 + +### Changed +- ルール数 21 → 20。 + +## [不採用の記録: 他リポジトリへの展開] + +10 リポ 1015 ファイルに対する実コーパス計測の結果、**展開を見送った**。 + +- 精度改善（下記 Fixed 参照）で指摘は 121 件 → 16 件（87% 削減）まで下がったが、 + 残った 16 件を全件目視した結果、行動を要するものは 0〜2 件だった。 +- 見送りの主因は精度ではなく、**対象コーパスに当該ルールが狙う脆弱性が + 残っていない**こと。指摘先はいずれも既に防御的に書かれていた + （`COVERAGE_COLUMNS` による識別子の固定、`_ALLOWED_WORKER_COLUMNS` の + allowlist、`SecurityHeadersMiddleware` の実装）。過去の CISO 監査と + レッドチームを通過済みのコードベースであり、後発のパターンスキャナに + 拾うものが無い状態だった。 +- 品質ルール（QUAL001/QUAL002）は同コーパスで 1310 件を出したため、 + 仮に展開する場合も無効化が前提になる。 + +教訓: ルールの妥当性は**自作の合成フィクスチャでは検証できない**。 +新ルールを追加したら、必ず実コードのコーパスに当てて誤検知率を数値で確認すること。 + 網羅性の検証で見つかった「README に書いてあるのに実装が無い」「0 件と見ていないが 区別できない」欠陥をまとめて是正した。検出ルールは 11 → 21 に増加。 diff --git a/QUICKSTART.md b/QUICKSTART.md index 7d4bc09..b6a4042 100644 --- a/QUICKSTART.md +++ b/QUICKSTART.md @@ -63,7 +63,6 @@ python validator.py --path . --config config/custom_rules.json - パスワードのハードコード - CORS設定の不備 - SQLインジェクションの可能性 -- セキュリティヘッダーの不足 ### コード品質問題 - 長すぎる行 diff --git a/README.md b/README.md index 5d1e751..0896404 100644 --- a/README.md +++ b/README.md @@ -14,7 +14,7 @@ | 🎯 **Use case** | Block insecure AI-generated code at PR time | | ⚡ **Speed** | <1s per file, `--git-diff` mode scans only changed files | | 🔒 **Privacy** | 100% offline. No code leaves your machine. Only dependency: `pydantic` | -| 🧪 **Detection rules** | 21 rules across security, quality, and dependency layers (SEC001–SEC013, QUAL001–QUAL002, DEP001–DEP006) | +| 🧪 **Detection rules** | 20 rules across security, quality, and dependency layers (SEC001–SEC013 excluding SEC007, QUAL001–QUAL002, DEP001–DEP006) | | 📦 **Install** | `pip install -r requirements.txt` — done | --- @@ -25,7 +25,6 @@ - Hardcoded credentials: API keys (OpenAI, Anthropic, Google, GitHub tokens), passwords, database URLs, Django/Flask `SECRET_KEY`, AWS access keys, and embedded PEM private keys - Dangerous CORS configurations: wildcard origins, and wildcard origins combined with `allow_credentials=True` - SQL injection patterns: f-string interpolation, `+` concatenation, `str.format()`, and `%` operator -- Missing security headers: `X-Content-Type-Options`, `X-Frame-Options`, `X-XSS-Protection` (FastAPI apps) - Command injection: `os.system` / `os.popen` / `subprocess(..., shell=True)` - Unsafe deserialization: `pickle` / `marshal` / `shelve` / `yaml.load` without a safe loader - Dynamic code execution: `eval` / `exec` (`ast.literal_eval` is excluded) @@ -50,7 +49,7 @@ API_KEY = "sk-..." # code-validator: ignore[SEC001] # code-validator: ignore-file # whole file, all rules ``` -File-level markers exist because some rules (CORS, security headers) are reported against the file rather than a single line. +File-level markers exist because some rules (CORS) are reported against the file rather than a single line. ### Reporting - **HTML**: human-readable browser report with color-coded severity cards @@ -166,7 +165,6 @@ Edit `config/validator_config.json` to customize behavior: "check_credentials": true, "check_cors": true, "check_sql_injection": true, - "check_security_headers": true, "check_dangerous_calls": true }, "quality_rules": { @@ -188,7 +186,6 @@ Edit `config/validator_config.json` to customize behavior: | `security_rules.check_credentials` | SEC001–SEC003, SEC008–SEC010 | | `security_rules.check_cors` | SEC004–SEC005 | | `security_rules.check_sql_injection` | SEC006 | -| `security_rules.check_security_headers` | SEC007 | | `security_rules.check_dangerous_calls` | SEC011–SEC013 | | `quality_rules.max_line_length` | QUAL001 threshold | | `quality_rules.check_unused_imports` | QUAL002 | @@ -275,7 +272,6 @@ code-validation: | SEC004 | Critical | Security | CORS wildcard origins + credentials enabled | | SEC005 | High | Security | CORS wildcard origins (production risk) | | SEC006 | High | Security | Potential SQL injection via f-string, `+`, `.format()`, or `%` | -| SEC007 | Medium | Security | Missing security header in FastAPI app | | SEC008 | Critical | Security | Hardcoded `SECRET_KEY` (Django / Flask session signing) | | SEC009 | Critical | Security | Hardcoded AWS access key or secret access key | | SEC010 | Critical | Security | PEM private key embedded in source | diff --git a/config/validator_config.json b/config/validator_config.json index a9eb38c..4990b56 100644 --- a/config/validator_config.json +++ b/config/validator_config.json @@ -22,7 +22,6 @@ "check_credentials": true, "check_cors": true, "check_sql_injection": true, - "check_security_headers": true, "check_dangerous_calls": true }, "quality_rules": { diff --git a/docs/dev/architecture.md b/docs/dev/architecture.md index c1453f3..41fd056 100644 --- a/docs/dev/architecture.md +++ b/docs/dev/architecture.md @@ -30,7 +30,7 @@ code-validator は **単一ファイルのモノリシック CLI** です。す | `Severity` | Enum | `class Severity` | 重大度 5 段階（critical / high / medium / low / info） | | `Issue` | dataclass | `class Issue` | 検出結果 1 件（severity / category / file_path / line_number / rule_id / message ほか） | | `ValidationResult` | dataclass | `class ValidationResult` | 1 回のスキャン結果全体（issues リスト + サマリー + 実行時間） | -| `SecurityScanner` | クラス | `class SecurityScanner` | SEC001–SEC013。認証情報 / CORS / SQLi / セキュリティヘッダー / 危険な API 呼び出しを検出 | +| `SecurityScanner` | クラス | `class SecurityScanner` | SEC001–SEC013 (SEC007 は削除済み)。認証情報 / CORS / SQLi / 危険な API 呼び出しを検出 | | `CodeQualityChecker` | クラス | `class CodeQualityChecker` | QUAL001–QUAL002。行長・未使用 import（関数複雑度はスタブで無検出） | | `DependencyChecker` | クラス | `class DependencyChecker` | DEP001–DEP006。`pip-audit` / `npm audit` へ委譲し、実行失敗を HIGH で報告 | | `_apply_suppressions` | 関数 | `def _apply_suppressions` | `# code-validator: ignore` 系コメントに一致する Issue を除外 | diff --git a/docs/llms.txt b/docs/llms.txt index e135a98..5f6a8f2 100644 --- a/docs/llms.txt +++ b/docs/llms.txt @@ -23,7 +23,6 @@ code-validator is a Python 3.9+ command-line tool that acts as a CI/CD quality g - SEC001–SEC003: Hardcoded credentials (API keys, passwords, DB URLs) - SEC004–SEC005: CORS wildcard + credentials misconfigurations - SEC006: SQL injection via f-string, concatenation, .format(), or % operator -- SEC007: Missing security headers in FastAPI apps - SEC008–SEC010: Hardcoded SECRET_KEY, AWS keys, embedded PEM private keys - SEC011–SEC013: Command injection, unsafe deserialization, eval/exec - QUAL001–QUAL002: Line length violations, unused imports diff --git a/llms.txt b/llms.txt index e135a98..5f6a8f2 100644 --- a/llms.txt +++ b/llms.txt @@ -23,7 +23,6 @@ code-validator is a Python 3.9+ command-line tool that acts as a CI/CD quality g - SEC001–SEC003: Hardcoded credentials (API keys, passwords, DB URLs) - SEC004–SEC005: CORS wildcard + credentials misconfigurations - SEC006: SQL injection via f-string, concatenation, .format(), or % operator -- SEC007: Missing security headers in FastAPI apps - SEC008–SEC010: Hardcoded SECRET_KEY, AWS keys, embedded PEM private keys - SEC011–SEC013: Command injection, unsafe deserialization, eval/exec - QUAL001–QUAL002: Line length violations, unused imports diff --git a/plan.md b/plan.md index 41baa74..569a082 100644 --- a/plan.md +++ b/plan.md @@ -7,7 +7,7 @@ ## フェーズ ### Phase 1: 初期実装 (完了) - validator.py メインスクリプト -- セキュリティルール (SEC001-SEC007) +- セキュリティルール (SEC001-SEC013) - コード品質ルール (QUAL001) - 依存関係ルール (DEP001-DEP003) - HTML/JSON レポート生成 diff --git a/spec.md b/spec.md index 24e55b1..a8786b5 100644 --- a/spec.md +++ b/spec.md @@ -11,7 +11,8 @@ AI生成コードのセキュリティ脆弱性・コード品質・依存関係 - [x] 機能1: セキュリティスキャン（API鍵・パスワード・DB URL・SECRET_KEY・AWS鍵・秘密鍵ブロック） - [x] 機能2: CORS設定不備の検出（ワイルドカードオリジン + credentials） - [x] 機能3: SQLインジェクションパターン検出（f文字列 / 連結 / .format() / % 演算子） -- [x] 機能4: セキュリティヘッダー不足の検出（FastAPIアプリ向け） +- [ ] ~~機能4: セキュリティヘッダー不足の検出~~ — 削除 (SEC007)。ヘッダーは別モジュールの + ミドルウェアに置く構成が標準で、行単位スキャナでは原理的に判定できないため - [x] 機能5: コード品質チェック（行長超過・未使用import） - 関数複雑度はスタブであり検出を行わない（実装予定なし。外部ツール併用を想定） - [x] 機能6: 依存関係監査（pip-audit / npm audit 委譲） diff --git a/tests/test_config.py b/tests/test_config.py index bcaa0ca..5dfee8f 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -35,7 +35,6 @@ "SEC004", ), ("check_sql_injection", 'cur.execute(f"SELECT * FROM t WHERE x = {y}")', "SEC006"), - ("check_security_headers", 'app = FastAPI()', "SEC007"), ("check_dangerous_calls", 'v = eval(expr)', "SEC013"), ], ) diff --git a/tests/test_security_headers.py b/tests/test_security_headers.py deleted file mode 100644 index 266f387..0000000 --- a/tests/test_security_headers.py +++ /dev/null @@ -1,90 +0,0 @@ -"""SEC007 (FastAPI のセキュリティヘッダー欠落) の回帰テスト.""" -# code-validator: ignore-file -from __future__ import annotations - -from validator import Severity - -from .conftest import assert_fires, assert_silent - - -FASTAPI_APP = '''from fastapi import FastAPI - -app = FastAPI() -''' - -# FastAPI に言及しているが、アプリを生成していないファイル。 -# 実コーパスの計測では、この形（バッジ文字列や型注釈のための import）が -# SEC007 誤検知の最大の発生源だった（72 件中 18 件が README 生成スクリプト）。 -FASTAPI_MENTIONED_ONLY = '''BADGES = { - "FastAPI": "![FastAPI](https://img.shields.io/badge/FastAPI-009688)", -} - - -def pick(tech_stack): - return [t for t in tech_stack if t in ["FastAPI", "Flask", "Django"]] -''' - -# ルーター側のモジュール。アプリ本体ではないのでヘッダーの責務を持たない。 -FASTAPI_ROUTER = '''from fastapi import APIRouter - -router = APIRouter() - - -@router.get("/items") -async def items(): - return [] -''' - -FASTAPI_WITH_HEADERS = '''from fastapi import FastAPI - -app = FastAPI() - - -@app.middleware("http") -async def add_headers(request, call_next): - response = await call_next(request) - response.headers["X-Content-Type-Options"] = "nosniff" - response.headers["X-Frame-Options"] = "DENY" - response.headers["X-XSS-Protection"] = "1; mode=block" - return response -''' - - -def test_sec007_reports_missing_headers_once(scan) -> None: - """アプリ 1 つにつき 1 件にまとめ、欠落ヘッダーを列挙すること. - - 旧実装はヘッダーごとに 1 件出していたため、ルーター分割したアプリで - 件数が膨らんだ（実コーパスで 72 件）。 - """ - issues = scan(FASTAPI_APP) - hits = assert_fires(issues, "SEC007") - assert len(hits) == 1, f"1 件にまとめる想定だが {len(hits)} 件: {[i.message for i in hits]}" - assert hits[0].severity == Severity.MEDIUM - assert "X-Content-Type-Options" in hits[0].message - assert "X-Frame-Options" in hits[0].message - - -def test_sec007_does_not_require_deprecated_xss_header(scan) -> None: - """非推奨の X-XSS-Protection は要求しないこと.""" - hits = assert_fires(scan(FASTAPI_APP), "SEC007") - assert "X-XSS-Protection" not in hits[0].message - - -def test_sec007_silent_when_headers_present(scan) -> None: - """ヘッダーが設定済みなら発火しないこと.""" - assert_silent(scan(FASTAPI_WITH_HEADERS), "SEC007") - - -def test_sec007_only_applies_to_fastapi(scan) -> None: - """FastAPI を使っていないファイルには適用しないこと.""" - assert_silent(scan('def main():\n return 1\n'), "SEC007") - - -def test_sec007_requires_app_instantiation(scan) -> None: - """FastAPI に言及しているだけのファイルでは発火しないこと（実コーパス由来）.""" - assert_silent(scan(FASTAPI_MENTIONED_ONLY), "SEC007") - - -def test_sec007_skips_router_modules(scan) -> None: - """APIRouter のみのモジュールはアプリ本体ではないので対象外.""" - assert_silent(scan(FASTAPI_ROUTER), "SEC007") diff --git a/tests/test_suppressions.py b/tests/test_suppressions.py index c76c3ce..335530e 100644 --- a/tests/test_suppressions.py +++ b/tests/test_suppressions.py @@ -56,7 +56,7 @@ def test_file_ignore_suppresses_everything(scan) -> None: def test_file_ignore_with_rule_ids_suppresses_only_those(scan) -> None: """ファイル単位でもルールを絞れること（line_number を持たないルール向け）.""" code = ( - '# code-validator: ignore-file[SEC004,SEC005,SEC007]\n' + '# code-validator: ignore-file[SEC004,SEC005]\n' 'from fastapi import FastAPI\n' 'from fastapi.middleware.cors import CORSMiddleware\n' 'app = FastAPI()\n' @@ -64,7 +64,7 @@ def test_file_ignore_with_rule_ids_suppresses_only_those(scan) -> None: f'K = "{FAKE_KEY}"\n' ) issues = scan(code) - for suppressed in ("SEC004", "SEC005", "SEC007"): + for suppressed in ("SEC004", "SEC005"): assert_silent(issues, suppressed) # 指定外のルールは生きている assert_fires(issues, "SEC001") diff --git a/validator.py b/validator.py index 5e5e6f1..d233a6b 100644 --- a/validator.py +++ b/validator.py @@ -284,7 +284,8 @@ def __init__(self, config: Optional[Dict[str, Any]] = None): self.check_credentials = rules.get('check_credentials', True) self.check_cors = rules.get('check_cors', True) self.check_sql_injection = rules.get('check_sql_injection', True) - self.check_security_headers = rules.get('check_security_headers', True) + # check_security_headers は SEC007 の削除にともない廃止。 + # 既存の設定ファイルに残っていても無視するだけで、エラーにはしない。 self.check_dangerous_calls = rules.get('check_dangerous_calls', True) def scan_file(self, file_path: Path) -> List[Issue]: @@ -308,10 +309,6 @@ def scan_file(self, file_path: Path) -> List[Issue]: if self.check_sql_injection: issues.extend(self._scan_sql_injection(file_path, lines)) - # セキュリティヘッダーの検出 - if self.check_security_headers: - issues.extend(self._scan_security_headers(file_path, lines)) - # 危険な動的実行・逆シリアライズ・シェル実行 if self.check_dangerous_calls: issues.extend(self._scan_dangerous_calls(file_path, lines)) @@ -541,44 +538,19 @@ def _scan_sql_injection(self, file_path: Path, lines: List[str]) -> List[Issue]: return issues - # FastAPI アプリの生成箇所。`FastAPI` という語の出現ではなく、 - # インスタンス化していることを条件にする。 + # SEC007（FastAPI のセキュリティヘッダー欠落）は削除した。 # - # 旧実装は `'FastAPI' in content` で判定していたため、実コーパスでは - # バッジ文字列に "FastAPI" を含むだけの README 生成スクリプトや、 - # 型注釈のためだけに import しているモジュールで大量に誤検知した。 - FASTAPI_APP_RE = re.compile(r'\bFastAPI\s*\(') - - # セキュリティヘッダーはアプリ全体で一度ミドルウェアに設定するもの。 - # X-XSS-Protection は現在非推奨（設定しないことが推奨）なので要求しない。 - REQUIRED_SECURITY_HEADERS = ('X-Content-Type-Options', 'X-Frame-Options') - - def _scan_security_headers(self, file_path: Path, lines: List[str]) -> List[Issue]: - """セキュリティヘッダーの検出""" - issues = [] - file_content = '\n'.join(lines) - - if not self.FASTAPI_APP_RE.search(file_content): - return issues - - missing = [h for h in self.REQUIRED_SECURITY_HEADERS if h not in file_content] - if not missing: - return issues - - # ヘッダーごとに 1 件ずつ出すとルーター分割したアプリで件数が膨らむため、 - # アプリ 1 つにつき 1 件にまとめる。 - issues.append(Issue( - severity=Severity.MEDIUM, - category="security", - file_path=str(file_path), - line_number=None, - message=f"セキュリティヘッダーが設定されていません: {', '.join(missing)}", - rule_id="SEC007", - suggestion="アプリ生成箇所でセキュリティヘッダーミドルウェアを追加してください", - )) + # 理由: 行単位・ファイル単位のスキャナでは原理的に判定できない。 + # セキュリティヘッダーは専用のミドルウェアモジュールに置き、アプリ生成箇所で + # add_middleware する構成が標準であり、ヘッダー名は定義側のファイルにしか + # 現れない。実コーパスでの計測では、正しくミドルウェアを実装しているアプリを + # 「未設定」と誤って指摘した（ORICON-Chart-Dashboard/web/app.py。実際は + # web/middleware.py の SecurityHeadersMiddleware で設定済み）。 + # + # 誤検知を絞り込む方向では解決できず、必要なのはモジュール横断の解析。 + # それはこのツールの設計範囲外なので、ルールごと削除する。 + # 同種の検査が必要な場合は Semgrep / CodeQL を使うこと。 - return issues - def _scan_dangerous_calls(self, file_path: Path, lines: List[str]) -> List[Issue]: """危険な API 呼び出しを検出する。