[Orange-fix] [code_quality/high] スキップジョブの jobRepo.Create エラーが静かに無視されている#377
Conversation
There was a problem hiding this comment.
🍊 Orange Codens レビュー
致命的: mockWorkflowJobRepo 型が未定義 — 新規テストとヘルパーで参照されていますが定義が存在せず、コンパイルエラーになります。failOnCreateJobRepo が別箇所に定義されていますが型名が異なり、代わりに使えません。
ci_worker.go の本修正 (skip パスの _ = w.jobRepo.Create → エラー return) は正しく、running path と整合性が取れています。
テスト側に不要な dead code (runWorkflowRecordingWithFailingJobRepo, failOnCreateJobRepo) が混在しています。
🔒 セキュリティ: 本PRはスキップジョブのDB登録時のエラーが無視されていた問題を修正するバグフィックスであり、OWASP Top 10 2023 / CWE Top 25 に該当する脆弱性の挿入・露出は確認されなかった。修正内容 (エラーの return による伝播) は信頼性向上であり、暗号誤用 / 認可漏れ / SSRF / 入力検証不足 / インジェクション等のセキュアコーディング項目には該当しない。diff 内で新規に定義された ErrSkipPathCreateFailure はプロダクションコード上でラップ等に利用されずテストモック専用の死コードとなっているが、これはセキュリティ影響のないコード品質事項のため指摘外とする。
🔴 critical: 1 / 🔵 low: 2
その他の指摘 (2 件)
- 🔵 [low]
backend/internal/worker/ci_worker_test.go:414—runWorkflowRecordingWithFailingJobRepoが未使用 (dead code) (confidence: 0.80,maintainability.deadCode.unusedDefinition) - 🔵 [low]
backend/internal/worker/ci_worker_test.go:454—failOnCreateJobRepoが未使用 (dead code) (confidence: 0.80,maintainability.deadCode.unusedDefinition)
head: f7b16bb | 🍊 Reviewed by Orange Codens — AI code review & security audit
| // fails to create or complete a skipped job during CIRun, the entire run | ||
| // fails with ErrSkipPathCreateFailure rather than silently succeeding. This | ||
| // guards against the skip path silently swallowing DB errors. | ||
| func TestCINeedsSkipJobRepoCreateErrorsFailsRun(t *testing.T) { |
There was a problem hiding this comment.
🔴 [critical] mockWorkflowJobRepo 型が diff 上に未定義でコンパイルエラーになる
新規追加されたテスト TestCINeedsSkipJobRepoCreateErrorsFailsRun およびヘルパー runWorkflowRecordingWithJobRepo で mockWorkflowJobRepo が参照されていますが、この型 (とそのメソッド) の定義が diff に含まれていません。
該当箇所:
// (start_line 252)
mockJobRepo := &mockWorkflowJobRepo{failOnCreate: true}
...
func runWorkflowRecordingWithJobRepo(t *testing.T, jobRepo *mockWorkflowJobRepo, yamlSrc string) ([]string, string, error) {同ファイルには failOnCreateJobRepo が別途定義されていますが、型名が異なります。mockWorkflowJobRepo の定義がない場合、テストファイルはコンパイルエラーになります。
修正方法:
mockWorkflowJobRepo型とCreate/CreateBatch/Completeメソッドの定義を diff に追加する、またはmockWorkflowJobRepoをfailOnCreateJobRepoに置き換える
codeQuality.typescript.undefinedSymbol | confidence: 0.90
Summary
Type of Change
Testing
Checklist
Task Goal
backend/internal/worker/ci_worker.go:290-299 の以下の指摘を修正する。
依存失敗でスキップされるジョブの DB 登録 (
Create) で返り値が_ =により無視されています。同じ関数の実行時分岐 (running path) ではCreateエラー時にreturn fmt.Errorf("create workflow job: %w", createErr)として run 全体を失敗させています。この不一致により、スキップジョブの DB 登録に失敗してもエラーが検知されず、run は正常終了したことになります。該当箇所:
対比 (running path でエラー時に return する箇所):
併せて該当箇所の regression test を追加する (test と impl を 1 task で完結させる)。
予定された差分ファイル
Task ID:
019f3a8c-cdb7-76ba-9895-ff8b89d27b87Generated by Purple Codens