Skip to content

feat: Phase 3実装完了 - 画像ZIP取得機能#4

Merged
hidezoh merged 4 commits intomainfrom
claude/issue-3-20251203-0059
Dec 3, 2025
Merged

feat: Phase 3実装完了 - 画像ZIP取得機能#4
hidezoh merged 4 commits intomainfrom
claude/issue-3-20251203-0059

Conversation

@hidezoh
Copy link
Owner

@hidezoh hidezoh commented Dec 3, 2025

概要

Phase 3: 画像ZIP取得機能を実装しました。

実装内容

  • 7サイト対応の画像ZIP URLパターン設定
  • ダウンロード・解凍・保存の統合機能
  • データベーステーブル自動作成
  • エラーハンドリング・リトライ機能
  • コマンドラインツール

ファイル変更

  • src/lib/ad-materials.ts: 核となる実装
  • scripts/download-ad-images.js: CLIツール
  • package.json: 依存パッケージ追加

Generated with Claude Code

- 7サイト対応の画像ZIP URLパターン設定
- ダウンロード・解凍・保存の統合機能
- データベーステーブル自動作成(ad_materials, ad_images)
- エラーハンドリング・リトライ機能
- 画像メタデータ取得(寸法・ファイルサイズ)
- コマンドラインツール(scripts/download-ad-images.js)
- ES Modulesサポート、リンティングエラー修正

Co-authored-by: Hideyuki Kato <hidezoh@users.noreply.github.com>
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

* 例: node scripts/download-ad-images.js "113019_002" "caribbeancom"
*/

import { downloadImageMaterials, AD_SITES } from '../src/lib/ad-materials.js';

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Point CLI import at available module

Running node scripts/download-ad-images.js fails immediately with ERR_MODULE_NOT_FOUND because it imports ../src/lib/ad-materials.js, but the repo only contains the TypeScript source (ad-materials.ts) and tsconfig sets "noEmit": true, so no .js is produced. The new CLI therefore cannot run as documented until the import targets a file that actually exists at runtime (e.g., the .ts source or a built artifact).

Useful? React with 👍 / 👎.

Comment on lines +190 to +194
if (!fs.existsSync(outputDir)) {
fs.mkdirSync(outputDir, { recursive: true });
}

zip.extractEntryTo(entry, outputDir, false, true);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Sanitize ZIP entry paths before extraction

Zip entry names are concatenated into outputDir and then passed straight to zip.extractEntryTo without normalizing or rejecting .. or absolute prefixes. If a fetched ZIP contains crafted entry names, downloadAndExtractImages will create directories and extract files outside outputBaseDir, enabling path traversal and overwriting arbitrary files whenever untrusted archives are processed.

Useful? React with 👍 / 👎.

GitHub Actions の npm ci エラーを解決:
- adm-zip, axios, sharp の依存関係を package-lock.json に追加
- npm install を実行して依存関係を同期

Fixes: npm ci エラー「package.json and package-lock.json are out of sync」
@github-actions
Copy link
Contributor

github-actions bot commented Dec 3, 2025

Claude encountered an error —— View job

Failed with exit code 1

I'll analyze this and get back to you.

@hidezoh
Copy link
Owner Author

hidezoh commented Dec 3, 2025

Mainブランチのワークフロー修正(Git LFS サポート追加)をPRに取り込むため、コメントでトリガーします。

修正内容:

  • GitHub Actions の に オプションを追加
  • これにより、Git LFSファイルがポインタとして正しく扱われます

@claude PRブランチにmainブランチの最新変更をマージしてください。

@github-actions
Copy link
Contributor

github-actions bot commented Dec 3, 2025

Claude encountered an error —— View job

Failed with exit code 1

I'll analyze this and get back to you.

@github-actions
Copy link
Contributor

github-actions bot commented Dec 3, 2025

Claude finished @hidezoh's task —— View job


Phase 3 画像ZIP取得機能のコードレビュー

Serena MCPを活用した包括的なコードレビューを実施しました。

タスクリスト

  • プロジェクト構造の理解(Serena onboarding)
  • 変更ファイルの詳細分析
  • セキュリティ脆弱性の調査
  • アーキテクチャ・設計の評価
  • データベース変更の確認
  • 技術品質の評価
  • 包括的フィードバックの提供

🚨 重要: 修正が必要な問題

1. 【緊急】セキュリティ脆弱性: パストラバーサル攻撃

ファイル: src/lib/ad-materials.ts:194

// 危険: ZIPエントリ名の検証なし
zip.extractEntryTo(entry, outputDir, false, true);

問題: 悪意のあるZIPファイルが ../../../etc/passwd のようなパスを含む場合、プロジェクトディレクトリ外にファイルが抽出される可能性があります。

修正案:

// パストラバーサル攻撃を防ぐ
const sanitizedFileName = path.basename(fileName);
if (sanitizedFileName !== fileName || fileName.includes('..')) {
  console.warn(`安全でないファイル名をスキップ: ${fileName}`);
  continue;
}
const outputPath = path.join(imageDirectory, sanitizedFileName);

2. 【緊急】CLIスクリプトの実行エラー

ファイル: scripts/download-ad-images.js:7

// エラー: .tsファイルを.jsとしてインポート
import { downloadImageMaterials, AD_SITES } from '../src/lib/ad-materials.js';

問題: TypeScriptファイル(.ts)を.jsとして参照しているため、ERR_MODULE_NOT_FOUNDエラーが発生します。

修正案:

import { downloadImageMaterials, AD_SITES } from '../src/lib/ad-materials.ts';

📊 アーキテクチャ・設計の評価

✅ 良い点

  1. 責任の分離: ダウンロード、解凍、データベース保存が適切に分離
  2. エラーハンドリング: リトライ機能と詳細なエラー処理
  3. 型安全性: TypeScriptの型定義が適切
  4. 設定管理: サイト設定の一元化

⚠️ 改善提案

  1. データベース接続管理:
// 現在: 関数ごとにDB接続を作成/破棄
const db = new Database(DB_PATH);
try { /* ... */ } finally { db.close(); }

// 推奨: 接続プールまたは共有接続の検討
  1. 設定の外部化:
// AD_SITESを環境変数または設定ファイルに移行検討

🗄️ データベース設計の評価

✅ 良い点

  1. 正規化: ad_materialsad_imagesの適切な分離
  2. インデックス: material_idにインデックス作成
  3. 制約: 適切なデフォルト値とタイムスタンプ

📝 データベーススキーマ

-- ad_materials テーブル
CREATE TABLE ad_materials (
  id INTEGER PRIMARY KEY AUTOINCREMENT,
  video_id TEXT NOT NULL,
  site_id TEXT NOT NULL,
  image_zip_url TEXT,
  image_zip_downloaded BOOLEAN DEFAULT 0,
  image_directory TEXT,
  embed_code TEXT,
  embed_code_available BOOLEAN DEFAULT 0,
  created_at DATETIME DEFAULT CURRENT_TIMESTAMP,
  updated_at DATETIME DEFAULT CURRENT_TIMESTAMP
);

-- ad_images テーブル
CREATE TABLE ad_images (
  id INTEGER PRIMARY KEY AUTOINCREMENT,
  material_id INTEGER NOT NULL,
  file_name TEXT NOT NULL,
  file_path TEXT NOT NULL,
  file_size INTEGER,
  width INTEGER,
  height INTEGER,
  created_at DATETIME DEFAULT CURRENT_TIMESTAMP
);

⚠️ 改善提案

  1. 外部キー制約: ad_images.material_idに外部キー制約追加を検討
  2. 一意制約: video_id + site_idの組み合わせに一意制約を検討

🛠️ 技術品質の評価

✅ 良い点

  1. Next.js 16対応: モジュール形式(ESM)の適切な使用
  2. 依存関係管理: 適切なパッケージ選択
  3. エラーハンドリング: 404エラー時のリトライ停止など賢い処理
  4. ログ出力: 詳細な進捗ログ

⚠️ 改善提案

  1. パフォーマンス: 大きなZIPファイル処理時のメモリ使用量
  2. テストカバレッジ: テストコードの追加が必要
  3. 型定義の改善:
// より厳密な型定義
export interface DownloadOptions {
  timeout: number;
  maxRetries: number;
  outputBaseDir: string;
}

// 結果型の改善
export type DownloadResult = 
  | { success: true; imageDirectory: string; extractedFiles: string[] }
  | { success: false; error: string };

🔍 コード品質の詳細

パフォーマンス考慮事項

  1. メモリ使用量: 大きなZIPファイルをメモリに全展開
  2. ファイルI/O: 画像メタデータ取得の最適化余地
  3. 並行処理: 複数画像の並行処理検討

セキュリティ考慮事項

  1. 入力検証: videoId, siteIdの検証強化
  2. ファイル権限: 抽出ファイルの権限設定
  3. リソース制限: ZIPファイルサイズ制限

🎯 Next.js 16 / アフィリエイト管理アプリ固有の観点

✅ 良い点

  1. App Router対応: ES Modules使用
  2. ファイル配置: src/libの適切な使用
  3. アフィリエイト機能: 7サイト対応の拡張可能な設計

📋 今後の拡張提案

  1. UI統合: Next.js UIでのダウンロード進捗表示
  2. API Routes: /api/download-imagesエンドポイント
  3. バックグラウンド処理: キューシステム導入

🚀 実装完了時のアクション項目

必須修正項目

  • パストラバーサル脆弱性の修正
  • CLIスクリプトのインポートパス修正
  • 入力検証の強化

推奨改善項目

  • テストコードの追加
  • エラーログの改善
  • パフォーマンス最適化
  • UI統合の検討

📝 総評

Phase 3の実装は機能的には優秀ですが、セキュリティ脆弱性とCLI実行エラーの修正が緊急で必要です。

強み:

  • 包括的な機能実装
  • 適切なアーキテクチャ設計
  • エラーハンドリング

改善点:

  • セキュリティ対策
  • テストカバレッジ
  • パフォーマンス最適化

修正後は非常に価値のある機能になると評価します。個人開発プロジェクトとしては適切なスコープと実装品質です。


@hidezoh
Copy link
Owner Author

hidezoh commented Dec 3, 2025

@claude 必須で修正が必要な部分を修正してください。

@github-actions
Copy link
Contributor

github-actions bot commented Dec 3, 2025

Claude finished @hidezoh's task —— View job


セキュリティ脆弱性とCLIエラーの修正完了 ✅

必須修正項目をすべて対応しました:

修正タスク

  • プロジェクト構造の確認
  • パストラバーサル脆弱性の修正(src/lib/ad-materials.ts:194
  • CLIスクリプトのインポートパス修正(scripts/download-ad-images.js:7
  • セキュリティテストの実装
  • TypeScript型エラー修正
  • リント・ビルドチェック実行
  • 修正内容のコミット・プッシュ

🛡️ セキュリティ修正内容

1. パストラバーサル脆弱性対策:

// セキュリティチェック追加(src/lib/ad-materials.ts:187-201)
const sanitizedFileName = path.basename(fileName);
if (sanitizedFileName !== fileName || fileName.includes('..') || path.isAbsolute(fileName)) {
  console.warn(`安全でないファイル名をスキップ: ${fileName}`);
  continue;
}

// 抽出先パス検証
if (!outputPath.startsWith(imageDirectory)) {
  console.warn(`安全でないパスをスキップ: ${fileName}`);
  continue;
}

2. CLIインポートエラー修正:

// scripts/download-ad-images.js:7
- import { downloadImageMaterials, AD_SITES } from '../src/lib/ad-materials.js';
+ import { downloadImageMaterials, AD_SITES } from '../src/lib/ad-materials.ts';

3. 堅牢性向上:

  • TypeScript型エラー修正(string | nullstring | undefined
  • セキュリティテストファイル追加(src/lib/__tests__/ad-materials-security.test.ts

✅ 品質チェック結果

  • Lint: 警告なし(ESLint通過)
  • Build: 成功(TypeScript正常コンパイル)
  • セキュリティ: パストラバーサル攻撃対策完了

🔒 対策された脆弱性

  • ../../../etc/passwd などの危険なパス
  • 絶対パス (/etc/shadow) の防止
  • ディレクトリ外への抽出防止

修正完了: PRの必須セキュリティ修正がすべて適用され、安全にZIP抽出機能を利用できるようになりました。

--- |

- パストラバーサル攻撃対策: ZIP抽出時のファイル名検証追加
- CLIスクリプト修正: インポートパスを.jsから.tsに変更
- セキュリティテスト追加: 脆弱性検出テストケース実装
- TypeScript型エラー修正: string|null → string|undefined対応

Co-authored-by: Hideyuki Kato <hidezoh@users.noreply.github.com>
@hidezoh hidezoh merged commit d59853a into main Dec 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants