fix(nightly-todo): tee による生 summary の Actions ログ露出を塞ぐ (ADR-072 決定 14、#369 feedback) - #370
Conversation
…369 post-merge feedback) 決定 14 は「公開面 = PR 本文」と狭く見ており、step ログを見落としていた。 Select task step は exe 出力を tee で selected.txt と画面 (= Actions ログ) の両方へ 出しており、そこに生の summary / target_files / caution 行が含まれていた。 **public repo では step ログも第三者に可視**なので、これは 381 の screening を 迂回する 2 つ目の公開面だった (#369 post-merge feedback Tier 1 #1、High)。 ## 修正 tee をリダイレクト (> selected.txt) に変え、ログへはマーカー行 (rank/branch/ledger のみ = 構造的に安全) と screening 済みの summary_display だけを grep で出す。生の 出力はファイルに留まり $GITHUB_OUTPUT 経由でのみ使われる (後段の許可リスト grep は不変)。 ## 実測 実データで確認: ログに出るのはマーカー行と summary_display (バッククォート置換済み) のみ。grep exit 0 で set -e でも落ちない。生 summary はファイルに残り GITHUB_OUTPUT 用途は保たれる。 ## 教訓 「公開面」は出力先を 1 つ塞ぐたびに次が見つかる (PR 本文 → step ログ)。棚卸しは 「PR 本文」で止めず経路単位で行う、と決定 14 へ追記した。narrow 修正が隣接エッジに 穴を作るパターン (memory dont-trust-takt-fix-output) の公開面版。 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughタスク選択結果を一時ファイルへ保存する方式へ変更しました。Actionsログには安全なマーカーとscreening済みの ChangesNightly Todo出力制御
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🤖 PR Monitor 分析 (GitHub Actions バックストップ)
Applicable Findings (Critical / High / Major)該当なし (レビュー指摘 0 件) Applicable Findings (Medium 以下)該当なし Filtered (not applicable)該当なし 次のアクション
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.github/workflows/nightly-todo.yml (1)
158-161: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win
exclude_ranksを環境変数経由で渡してください。Line 160 は
steps.inflight.outputs.exclude_ranksを shell 本文へ直接展開します。現在の前段処理は数字とカンマだけを出力するため、現時点で shell injection は確認できません。ただし、この制約が将来変更されると、出力値が shell 構文として解釈されます。
env: EXCLUDE_RANKS: ${{ steps.inflight.outputs.exclude_ranks }}を設定し、--exclude-ranks "$EXCLUDE_RANKS"を使用してください。推奨変更
+ env: + EXCLUDE_RANKS: ${{ steps.inflight.outputs.exclude_ranks }} run: | ... - --exclude-ranks "${{ steps.inflight.outputs.exclude_ranks }}" \ + --exclude-ranks "$EXCLUDE_RANKS" \🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/nightly-todo.yml around lines 158 - 161, Update the workflow step invoking cli-nightly-task-select to pass steps.inflight.outputs.exclude_ranks through an EXCLUDE_RANKS environment variable, then reference "$EXCLUDE_RANKS" for the --exclude-ranks argument instead of interpolating the output directly in the shell command.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/nightly-todo.yml:
- Around line 162-164: Split the task-selection CLI execution and the
selected.txt output validation into separate workflow steps. Keep
continue-on-error only on the CLI execution step, and remove it from the
validation step containing the grep so missing markers, summary_display, or
unreadable selected.txt fail the job instead of entering the no-op path
controlled by steps.select.outcome.
In `@docs/adr/adr-072-nightly-todo-loop.md`:
- Around line 284-286:
ADR-072のstepログとPR本文の保護方法を分けて記述してください。stepログについては、summary_displayを制御文字除去・1行化したうえで固定プレフィックス付きで出力する方法を明記し、PR本文についてはインラインコードスパンで保護する方法を明記してください。
---
Nitpick comments:
In @.github/workflows/nightly-todo.yml:
- Around line 158-161: Update the workflow step invoking cli-nightly-task-select
to pass steps.inflight.outputs.exclude_ranks through an EXCLUDE_RANKS
environment variable, then reference "$EXCLUDE_RANKS" for the --exclude-ranks
argument instead of interpolating the output directly in the shell command.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 06027964-98d5-4554-9036-236ec147504e
📒 Files selected for processing (2)
.github/workflows/nightly-todo.ymldocs/adr/adr-072-nightly-todo-loop.md
| # 可観測性: マーカー行 (rank/branch/ledger のみ = 構造的に安全) と screening 済みの | ||
| # summary_display だけをログへ出す。生の summary/target_files/caution は出さない。 | ||
| grep -E '^\[NIGHTLY_TASK\]|^summary_display=' "$RUNNER_TEMP/selected.txt" |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
出力契約の検証失敗を no-op として扱わないでください。
set -euo pipefail により、Line 164 の grep が [NIGHTLY_TASK] または summary_display= を見つけられない場合、Select task step は失敗します。しかし、この step の continue-on-error: true と後続の steps.select.outcome != 'success' により、出力契約違反や $RUNNER_TEMP/selected.txt の読み取り失敗が no-op 経路へ入ります。後続 step が 0 で終了するため、夜間処理の異常が green run に見えます。
選択 CLI の実行と出力検証を別 step に分け、出力検証 step には continue-on-error を付けずに失敗させてください。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/nightly-todo.yml around lines 162 - 164, Split the
task-selection CLI execution and the selected.txt output validation into
separate workflow steps. Keep continue-on-error only on the CLI execution step,
and remove it from the validation step containing the grep so missing markers,
summary_display, or unreadable selected.txt fail the job instead of entering the
no-op path controlled by steps.select.outcome.
| 公開面の棚卸し結果、台帳由来で外部可視になるのは **PR 本文の `内容`** と **step ログ**の 2 つだった。`RANK` は `u32` にパース済み、ブランチ名は `format!("claude/nightly-{rank}")` で、どちらも**構造的に安全**である。 | ||
|
|
||
| **初版は「公開面 = PR 本文」と狭く見ており、step ログを見落としていた** (#369 post-merge feedback が指摘)。`Select task` step は exe 出力を `tee` で `selected.txt` と**画面 (= Actions ログ) の両方**へ出しており、そこに生の `summary` / `target_files` / `caution` 行が含まれていた。**public repo では step ログも第三者に可視**なので、これは screening を迂回する 2 つ目の公開面だった。`tee` をリダイレクト (`> selected.txt`) に変え、ログへはマーカー行 (rank/branch/ledger のみ = 安全) と screening 済みの `summary_display` だけを `grep` で出す形にした。生の出力はファイルに留まり `$GITHUB_OUTPUT` 経由でのみ使われる。**「公開面」は出力先を 1 つ塞ぐたびに次が見つかる**ので、棚卸しは「PR 本文」で止めず経路単位で行う。 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
step ログと PR 本文の保護方法を分けて記述してください。
.github/workflows/nightly-todo.yml の Lines 162-164 は summary_display を summary_display= の後ろに直接ログ出力します。インラインコードスパンは同ファイルの Lines 486-490 の PR 本文でのみ使用します。
したがって、続く説明では、step ログは制御文字除去・1 行化・固定プレフィックスで保護し、PR 本文はインラインコードスパンで保護すると公開面ごとに明記してください。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/adr/adr-072-nightly-todo-loop.md` around lines 284 - 286,
ADR-072のstepログとPR本文の保護方法を分けて記述してください。stepログについては、summary_displayを制御文字除去・1行化したうえで固定プレフィックス付きで出力する方法を明記し、PR本文についてはインラインコードスパンで保護する方法を明記してください。
🤖 PR Monitor 分析 (GitHub Actions バックストップ)
Applicable Findings (Critical / High / Major)
Applicable Findings (Medium 以下)
Filtered (not applicable)
次のアクション
|
Resolved findings: - [Major] .github/workflows/nightly-todo.yml:164 出力契約の検証失敗を no-op として扱わないでください。 - [Minor] docs/adr/adr-072-nightly-todo-loop.md:286 step ログと PR 本文の保護方法を分けて記述してください。
…R-072 決定 14、#369/#370) 決定 14 は「公開面 = PR 本文」と狭く見ており step ログを見落としていた。Select task step は exe 出力を tee で selected.txt と画面 (= Actions ログ) の両方へ出しており、 生の summary/target_files/caution が含まれていた。**public repo では step ログも 第三者に可視**なので、381 の screening を迂回する 2 つ目の公開面だった (#369 post-merge feedback Tier 1 #1、High)。 ## tee → リダイレクト tee を > selected.txt に変え、生の出力はファイルに留めて $GITHUB_OUTPUT 経由でのみ 使う。ログへはマーカー行 (rank/branch/ledger のみ = 構造的に安全) と screening 済みの summary_display だけを出す。 ## 出力契約検証を別 step に分離 (#370 CodeRabbit Major 指摘) 初版は「マーカー欠落なら exit 1」を Select task step の中に置いていたが、同 step は exit 2 (台帳破損) / exit 3 (該当タスク無し) を区別するため continue-on-error が必要で、 その非 success は下段 Stop step が「正常な no-op」として green で終える。検証を同 step に 置くと、**exe は成功したのに出力が壊れているケースの exit 1 も continue-on-error に 飲まれ、出力契約違反が「今夜は何も無かった」に化ける**。 検証を Validate and echo the task-selection output contract step に切り出し、 continue-on-error を付けないことで job の red として顕在化させる (if: steps.select.outcome == 'success' なので exit 2/3 の no-op 経路とは分離)。 ## ADR の保護方法を公開面ごとに明記 (#370 CodeRabbit Minor 指摘) 決定 14 が step ログと PR 本文の保護方法を混ぜて書いていた。実際は screening 処理は 共通だが囲み方が違う (PR 本文 = コードスパン、step ログ = 固定プレフィックス付き 1 行)。 公開面ごとに分けて記述した。 ## 教訓 「公開面」は出力先を 1 つ塞ぐたびに次が見つかる (PR 本文 → step ログ)。棚卸しは 経路単位で行う、と決定 14 へ追記。 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
e13a957 to
aea194e
Compare
* docs(todo): WP-18 セッションの観測を順位 385-388 へ登録し todo21.md を新設 (Phase 2) WP-18 の PR 作業 (#364〜#370) で実測した自動化経路の運用問題を todo へ登録し、 384 完了削除・todo ローテーション・WP-11 記録を 1 バッチにまとめる。 ## todo21.md 新設 (todo20.md が 56KB = 50KB 閾値超過) 新規追加先を todo20.md → todo21.md へ移行。breadcrumb を持つ 5 ファイル (todo.md / todo8 / todo10 / todo13 / todo14) の「現在の追加先」ポインタと、 数詞「22つ/todo2-20」を持つ 8 ファイル (todo3-11) を 23つ/todo2-21 へ更新。 ## 順位 385-388 (2026-08-08 実測、todo21.md) - 385 (T3): cli-pr-monitor lock の liveness check 欠落 (復帰窓 30 分) - 386 (T2): 監視・自動 fix 経路の空コミットで bookmark ずれ → merge/push 失敗。 **本セッションで 7 回観測**、生成元確定、深さ非依存 revset が本命の対処 - 387 (T2): 自動 fix は push が BLOCK されてもローカル作業コピーを書き換える - 388 (T3): post-merge-feedback の完了判定が書き込みと race し誤 failed marker いずれも post-merge feedback には構造的に入らない (feedback の入力は PR diff と レビュー指摘で、ツール自身の運用中の事象は拾わない)。 ## 順位 384 完了・削除 外部設定の実体は ADR-072 § 外部設定の実体 に記録済み (#369/#370)。todo20.md の full エントリと summary2 の行を削除し、完了記録の 1 行に置換。 ## WP-11 記録 (harness-improvement-plan) #366 で enforce 下の scope guard 誤検知を 1 件観測。anchor と remedy が別ファイルの 指摘は構造的に必ず BLOCK される。本採用判定の前に判定基準の再定義が要ることを記録。 ## 検証 pnpm lint:docs OK (preamble + cross-ref + priority-inversion — 数詞 23 整合を含む) / markdownlint 127 files 0 error。 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs: CodeRabbit 指摘 4 件に対応 — 実装確認のうえ断定を訂正 (#371) いずれも「実コードを確認せず断定していた」箇所で、実装を verify して直した。 ## #1 (harness-plan) WP-11 の「誤検知」→「設計どおりの保守的 deny」 evaluate_scope_guard の allowlist は allowlist_from_paths(findings.map(f.file)) = finding の anchor 位置だけで、remedy が別ファイルなら含まない (ADR-054 も欠点として 明記)。#366 の BLOCK は誤検知ではなく設計どおりの保守的 deny。本採用の判定基準を 「この保守的 deny を誤検知に数えない」よう明確化する、と修正。 ## #2 (todo.md/todo3-7) breadcrumb の todo20/todo2-20 残存 docs バッチで更新し漏れた参照を補完。todo.md 冒頭の使い分けを todo21 + summary2 まで、 todo3-7 の「todo2-20」を todo2-21 へ。全 docs で todo2-20 残存ゼロを確認。 ## #3 (todo21:58) heads(::@ & bookmarks()) の複数返り @ に複数 bookmark が付くと複数コミットを返し clone --head / PR 選択が多対象になる。 trunk 除外 + 単一 bookmark へ絞る (現行 is_trunk_bookmark 除外と同規律) 必要を追記。 ## #4 (todo21:102) 388 の「race」断定を撤回 reconcile_takt_output → copy_feedback_report は find_latest_run_dir で run dir を 選ぶ (mod.rs:147 / takt.rs:84)。単純な write race と断定せず、latest 特定のずれ / パス不一致 / 前後関係を「まず特定する」形へ。#367 では実体が run dir に存在した。 ## 検証 pnpm lint:docs OK / markdownlint 0 error。scope guard・feedback reconcile の実装を 実際に読んで記述と一致させた。 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…Phase 2 C) WP-18 の prompt injection 対策 PR (#369/#370) の post-merge feedback の採用候補の うち、セッション中に未対応で価値の高い分をユーザー承認 (2026-08-09) のうえ登録する。 ## 新規登録 (todo21.md、389-391) - 389 (T1): Write(path) tool-scope 指定子の no-op を検出する settings validator。 CLI 2.1.218 で Write() は no-op = deny の silent 無効化。順位 379 で実際に踏んだ。 **検知は warning ではなく error (必須 CI 失敗) にする** — silent security failure は ADR-043 の fail-closed 対象 (#372 CodeRabbit 指摘) - 390 (T2): 台帳 framing 区切りの定数 (LEDGER_DATA_FRAME_MARKER) と workflow リテラル (===BEGIN/END_LEDGER_DATA===) の cross-file 一致を CI 検証 - 391 (T3): jj の落とし穴 (squash 方向・空コミットでの bookmark ずれ) を dev-conventions へ。本セッションで複数回踏んだ。**操作例は再現可能な最小の初期状態 つきで書く** (jj バージョン・リモート有無・コミットグラフ・bookmark 位置に依存する ため、断定形でなく前提つきで、#372 CodeRabbit 指摘) ## 順位 375 補強 (todo20.md) narrow-fix が隣接エッジに穴を作る教訓を 5 項目目として追加 (#369/#370 で複数回 再演、memory dont-trust-takt-fix-output と同根)。新規 rank は立てず既存 375 を編集。 ## 見送り (ユーザー非選択) Cf カテゴリ網羅テスト (#369 T1#3) / GITHUB_TOKEN vs App token 挙動差テスト (#364 T2#3) / pre-push warning 対応方針 (#370 T3#4) は今回登録しない。既に セッション中に対応済みの候補も再登録しない。 ## 検証 pnpm lint:docs OK / markdownlint 0 error。 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…Phase 2 C) (#372) WP-18 の prompt injection 対策 PR (#369/#370) の post-merge feedback の採用候補の うち、セッション中に未対応で価値の高い分をユーザー承認 (2026-08-09) のうえ登録する。 ## 新規登録 (todo21.md、389-391) - 389 (T1): Write(path) tool-scope 指定子の no-op を検出する settings validator。 CLI 2.1.218 で Write() は no-op = deny の silent 無効化。順位 379 で実際に踏んだ。 **検知は warning ではなく error (必須 CI 失敗) にする** — silent security failure は ADR-043 の fail-closed 対象 (#372 CodeRabbit 指摘) - 390 (T2): 台帳 framing 区切りの定数 (LEDGER_DATA_FRAME_MARKER) と workflow リテラル (===BEGIN/END_LEDGER_DATA===) の cross-file 一致を CI 検証 - 391 (T3): jj の落とし穴 (squash 方向・空コミットでの bookmark ずれ) を dev-conventions へ。本セッションで複数回踏んだ。**操作例は再現可能な最小の初期状態 つきで書く** (jj バージョン・リモート有無・コミットグラフ・bookmark 位置に依存する ため、断定形でなく前提つきで、#372 CodeRabbit 指摘) ## 順位 375 補強 (todo20.md) narrow-fix が隣接エッジに穴を作る教訓を 5 項目目として追加 (#369/#370 で複数回 再演、memory dont-trust-takt-fix-output と同根)。新規 rank は立てず既存 375 を編集。 ## 見送り (ユーザー非選択) Cf カテゴリ網羅テスト (#369 T1#3) / GITHUB_TOKEN vs App token 挙動差テスト (#364 T2#3) / pre-push warning 対応方針 (#370 T3#4) は今回登録しない。既に セッション中に対応済みの候補も再登録しない。 ## 検証 pnpm lint:docs OK / markdownlint 0 error。 Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Summary
Select taskstep のteeを リダイレクト(> selected.txt)に変更し、生のsummary/target_files/cautionが Actions ログへ露出する経路を塞ぐsummary_display)を出すContext
Why: #369 の post-merge feedback(Tier 1 #1、High)が指摘した実在の穴。決定 14(順位 381)で「公開面 = PR 本文」と棚卸しし screening を入れたが、同じ step の
teeが exe の全出力を画面(= Actions ログ)へも出しており、そこに生の台帳自由記述が含まれていた。public repo では step ログも第三者に可視なので、screening を迂回する 2 つ目の公開面が残っていた。Fix:
tee→> selected.txtに変更。ログへはマーカー行(rank/branch/ledger のみ = 構造的に安全)とsummary_display(screening 済み)だけをgrepで出す。生の出力はファイルに留まり$GITHUB_OUTPUT経由でのみ使われる(後段の許可リスト grep は不変)。教訓: 「公開面」は出力先を 1 つ塞ぐたびに次が見つかる(PR 本文 → step ログ)。棚卸しは経路単位で行う、と決定 14 へ追記。narrow 修正が隣接エッジに穴を作るパターンの公開面版。
Validation
summary_display(バッククォート置換済み)のみ。grepexit 0 でset -eでも落ちない。生summaryはファイルに残り$GITHUB_OUTPUT用途は保たれるpnpm pushpre-push review: verdict=APPROVE(両 facet、warning なし)pnpm lint:docs: OKReferences
Summary by CodeRabbit