feat: composite action として VRT Action を実装 - #1
Conversation
VRT へのスクリーンショット/Storybook 送信を composite action として実装した。 TypeScript ではなくランナー上の curl/jq/git/tar だけで完結させ、bundle した dist/ をコミットせずレビュー可能な状態を保つ方針。 - action.yml: inputs/outputs/composite step。全処理を単一 step で実行し、 outputs を書き出してから exit することで、continue-on-error 利用時も build-url を参照できるようにした - scripts/lib.sh: 共通ヘルパー(マスク済み curl --config での Bearer 認証、 status→exit-code マッピング、build-url 組み立て、outputs 書き出し) - scripts/validate.sh: 入力の解決と検証。mode 不正、only-changed+screenshots、 stats-json 単独指定、project 形式不正、token/url 空をエラーにする - scripts/collect-pngs.sh: PNG 走査。PNG 以外・空・25MiB 超をアップロード前に 検出し、dir 相対パスから .png を除いた名前を導出する - scripts/screenshots.sh: CI REST API を直接叩く(作成→アップロード→finalize→ wait ポーリング、タイムアウト30分で exit 2) - scripts/storybook.sh: vrt CLI に委譲。Release から OS/arch 別バイナリを取得し .sha256 で検証、--json の最終行を解釈して outputs に反映 - 追加 input: cli-version(storybook 用 CLI タグ)、app-url(build-url のベース) - commit 既定値を PR head SHA 優先に変更(pull_request の GITHUB_SHA は永続 しないマージコミットのため、コミットステータスを貼れる head SHA を使う) - README を実装済み前提に更新し、追加 input と挙動を追記
`die` は終了コードを `: "${CODE:=2}"` で決めていたが、main.sh が dispatch 前に
CODE=0 を初期化しているため代入が発火せず、ビルド作成後の致命的エラーでも
exit 0 で終了していた。outputs には result=failed / exit-code=0 という矛盾した
値が書かれ、アップロード失敗や HTTP エラーでもワークフローは緑のまま通る。
終了コードを条件付きでなく明示的に代入するよう修正する。ビルド作成後の失敗は
2(README の failed 相当)、作成前および入力エラーは 1。
ビルド作成後の失敗 → exit 2 / result=failed / exit-code=2
ビルド作成前の失敗 → exit 1
入力検証エラー → exit 1
の 3 経路を実際に実行して確認した。
あわせて SC2034 の誤検知に対する shellcheck ディレクティブを追加する。これらの
スクリプトは main.sh が source して変数を共有する構造のため、ファイル単位の
解析では未使用に見えるもので、実際には使われている。将来 CI に shellcheck を
入れたときに本物の指摘が埋もれないようにする。
|
ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing |
コードレビュー総合判定: APPROVE with Comments シェルで書かれた composite action としては、危険な箇所がよく押さえられています。特に inputs を 指摘は 3 件です。うち 2 件は「想定外の入力・外部要因を黙って通してしまう」型で、いずれも回帰検知ゲートとしての信頼性に関わります。 🟡 Low-Medium:
|
- wait / only-changed の真偽値検証を validate_inputs に追加。mode と同じ流儀で
true/false 以外は die する。誤記を黙って false に倒すと、wait の場合は回帰が
あってもワークフローが緑で通ってしまうため。
- cli-version: latest の解決を認証付き・--fail 付きにし、prerelease/draft を除外
してタグ選別する。未認証(60回/時/IP)・--fail 無しで 403 の JSON が jq に流れて
誤診する問題を解消。github-token input(既定 ${{ github.token }})を追加し、空文字
(フォーク PR 等)ならヘッダを付けず未認証で試みる。curl 失敗(レート制限・ネット
ワーク・認証)と「フィルタ結果が空」でエラーメッセージを分けた。
レビュアー提案の /releases/latest には切り替えなかった。このリポジトリには将来
バックエンド本体の Release も tag prefix で並ぶ設計のため、/releases/latest は
バックエンドの Release が最新になった時点で常に cli-v* 以外を返して死ぬ。一覧 +
フィルタなら prerelease 除外とタグ選別を両立できる。
- die を常に write_outputs を通す形にし、ビルド作成前の使い方エラーでも
result=failed / exit-code が書かれるようにした。BUILD_ID の有無で終了コードを
1(使い方の誤り)/2(実行時の失敗)に区別する。write_outputs は全フィールドが
${VAR:-}、出力先が ${GITHUB_OUTPUT:-/dev/null} で防御済みのため、GITHUB_OUTPUT が
無い早期の die でも set -u 下で二次エラーにならない。README の outputs 記述
(終了コードにかかわらず書き出す)と実挙動が一致するようになった。
Ubuntu ランナーの bash 5 では問題にならないが、bash 4.3 以前
(macOS ランナーの /bin/bash 等)では set -u 下で空配列の
"${auth[@]}" が unbound variable エラーになる。
${auth[@]+...} 形式で空のときは何も展開しないようにする。
- screenshots: name は --form-string で渡し、PNG は mktemp パスへコピーして から -F に渡す。@/< 始まりの名前がファイル参照として解釈され、パス中の ; や引用符が form 構文として解釈される curl の挙動を遮断する - collect-pngs: GNU 専用の sort -z をやめ、NUL 区切りで収集した配列を シェル内挿入ソート(LC_ALL=C)で並べる。BSD sort しかない macOS でも動き、 改行入りパスも壊れない - lib: curl に --connect-timeout/--max-time を追加。poll 中の一時的な transport エラーは try_request で deadline まで再試行し、 create/upload/finalize は従来どおり即失敗させる - storybook: CLI が JSON を吐かない場合、pipefail で die に到達する前に スクリプトが即死し outputs も案内も出なかったのを修正(テストで発見)
- test-validate: 入力検証の受理/拒絶 11 ケース - test-collect-pngs: 名前導出・バイト順ソート・非PNG/空/25MiB 超過の拒絶 - test-screenshots: fake API server に対する main.sh の e2e。@ < ; 引用符・ 改行入りの名前がそのまま届くこと、ファイルバイトの完全性、outputs、 changes_detected の exit 1、wait:false を検証 - test-storybook: download_cli を stub した fake CLI で引数構築・env 契約・ JSON 抽出・exit code 対応を検証 - CI: ubuntu で bash -n + shellcheck、ubuntu/macos matrix でテスト実行
HTTPServer.server_bind() の socket.getfqdn() が macOS ランナーで数十秒 ブロックし、port の出力が起動待ちの 10 秒を超えていた。server_bind を TCPServer のものに差し替えて getfqdn を踏まないようにし、起動待ちを 30 秒へ延長、失敗時は server の stderr を出力するようにした
- collect-pngs: 拡張子とサイズだけだった事前検証を実 API の制約に合わせた。 PNG シグネチャと IHDR の寸法(1〜10000px)、trim 後の名前が 1〜255 バイト、 trim 後の名前のビルド内重複(foo.png と foo.PNG の衝突を含む)を、 ビルド作成前に全ファイルへ適用する。壊れた PNG や重複名で 未 finalize のビルドが残る経路を塞ぐ - storybook: release 一覧を per_page=100 でページングし、安定版 cli-v* が 見つかるか空ページに達するまで走査する(上限 20 ページ)。既定 30 件の 1 ページ目にバックエンド Release が並ぶと cli-v* を見失う問題の修正。 API ベース URL は GITHUB_API_URL を尊重(GHES 対応・テスト差し替え可能) - tests: 正常系をデコード可能な実 PNG に差し替え。壊れた PNG・寸法超過・ 空名・重複名・255 バイト超過の拒絶テストと、2 ページ目の cli-v* を 見つけるページングテスト(prerelease 除外・pinned 短絡・不在時 die)を追加
macOS の APFS では foo.png と foo.PNG が同一ファイルになり、重複自体が
発生せずテストが失敗していた。移植可能な trim 衝突('foo' と 'foo ')を
主ケースにし、大文字小文字の衝突はファイルが 2 つ作れた場合のみ検査する
サーバー側 validate_png() は image::into_dimensions() を使い、png crate は 署名から最初の IDAT の直前まで全チャンクをパースして CRC も検証する (IDAT 以降は読まない)。従来の先頭 24 バイト検査では、有効な署名と寸法を 持つが途中で切れたファイルが事前検証を通過し、ビルド作成後のアップロードで 拒否されて未 finalize のビルドが残っていた。 - lib: png_dimensions を同じ区間のチャンクウォークに置き換え。チャンク長の 境界検査と CRC-32(純 bash、対象は pre-IDAT 領域のみで実質 1KiB 未満)を 検証し、失敗理由を die のメッセージへ含める。pre-IDAT が 256KiB を超える・ 100 チャンクを超える異常系は明示的に拒否 - tests: IHDR 途中(24 バイト)切断・IDAT 前切断・IHDR の CRC 破損・ 幅 0 の拒絶テストを追加。寸法超過テストは CRC まで正しいヘッダを生成する write_png_dims(python3)に差し替え。ancillary チャンクと複数 IDAT を含む 現実的な PNG が通過することはローカルで確認済み
CRC とチャンク境界だけでは、CRC を再計算した不正 IHDR(未定義 color type 等)が 事前検証を通過し、サーバーの png crate に拒否されて未 finalize ビルドが残る。 png crate が IDAT 到達前に確実に弾く意味検査を追加する: - bit depth / color type の許可される組み合わせ(PNG 仕様の表どおり) - compression / filter method は 0 のみ、interlace method は 0/1 のみ - 重複 IHDR の拒否 - IHDR/PLTE/IDAT/IEND 以外の critical chunk(型 1 バイト目の bit5 が 0)の拒否。 未知の ancillary chunk は crate と同様に素通しする - PLTE 長の検査(3 の倍数・最大 768 バイト) tests: 上記それぞれの拒絶ケース 7 件と、過剰検査でないことを確認する受理 ケース(Adam7 interlace・未知 ancillary chunk)を追加。write_png_dims を IHDR 全フィールドと追加チャンクを指定できる形に拡張
png crate は pre-IDAT で 2 つ目の PLTE を DuplicateChunk として拒否し、 fcTL は ancillary ながらパース失敗を例外的に致命扱いする。どちらも 素通しだと、CRC が正しい不正 PNG がアップロード時に拒否され 未 finalize ビルドが残る。 - lib: PLTE の出現を記録して重複を拒否。fcTL は長さ 26・pre-IDAT では 1 個のみ・sequence 0・フレーム矩形が画像内・dispose(0-2)/blend(0-1) を検証 - tests: 重複 PLTE と CRC が正しい不正 fcTL(長さ・sequence・矩形・ blend op・重複)の拒絶 6 件、単一 PLTE + 正常 fcTL の受理 1 件を追加。 write_png_dims の EXTRA を TYPE[:HEXDATA] のカンマ区切りへ拡張
実行時に取得する外部バイナリへ、継承した INPUT_GITHUB_TOKEN / GITHUB_TOKEN 等の資格情報を渡さない。CLI へ渡すのは PATH と VRT_URL / VRT_TOKEN / VRT_PROJECT のみ。テストには canary 環境変数が CLI へ漏れないことの検証を 追加し、fake CLI は env 非依存(パス焼き込み)へ変更した
レビュー指摘は「既知 ancillary chunk (tRNS/sBIT/acTL 等) を意味検査せず、 サーバーで拒否される」だったが、png 0.18.1 のソース確認と read_info への 細工 PNG 投入 (33 ケース) で実測したところ、前提が逆だった: - parse_chunk は fcTL 以外の ancillary chunk の Format エラーを BadAncillaryChunk として握り潰して続行する。RGBA への tRNS も短い grayscale tRNS も不正 sBIT/acTL/bKGD/gAMA もサーバーは受理する。 - skip_ancillary_crc_failures が既定で有効なため、ancillary chunk の CRC 不一致もチャンクごとスキップされる(従来実装はここで拒否していて サーバーより厳しすぎた)。 - PLTE の長さ制約は start_chunk の 3..=768 のみで、3 の倍数かは見ない (長さ 4 の PLTE は受理される。従来実装は過剰拒否)。 - 唯一の本物の緩さは fcTL: pre-IDAT の fcTL は validate_default_image に より x/y=0 かつ IHDR と同寸が必須(従来の「画像内に収まる」では不足)。 また sequence が 0 からの連番なら複数の fcTL も合法で、CRC の壊れた fcTL は連番に数えられない。長さ 26 以外は CRC より先に致命。 png_dimensions をこの境界どおりに書き換え、critical chunk のみ CRC を 検証、fcTL 以外の ancillary は無検査スキップとした。テストは実測で確定 した受理/拒否の全境界(PLTE 長 4/0/769 超、tRNS on RGBA、CRC 破損 ancillary、CRC 破損 fcTL、連番 fcTL、部分フレーム fcTL 等)を回帰として 追加し、helpers.sh の write_png_dims に :badcrc 記法を足した。 検証: bash 実装と png 0.18.1 read_info に同一の 33 PNG を与え、判定が 全件一致することを確認。tests/run.sh 全件 PASS。
resolve_cli_tag が -H "Authorization: Bearer $GITHUB_TOKEN" を curl の 引数に展開していたため、実行中は ps や /proc/*/cmdline からトークンを 読めた。VRT トークンと同じ方式に揃え、init_github_auth_config が書く mode 600 の curl config (GH_AUTH_CONFIG) 経由でヘッダーを渡す。VRT 用の AUTH_CONFIG とは別ファイルで、GitHub API リクエストにだけ指定する。 trap EXIT は上書き式で、AUTH_CONFIG と GH_AUTH_CONFIG が別々に trap を 張ると片方の削除が消えるため、lib.sh に register_tmp_cleanup を新設して 単一 trap に集約した。resolve_cli_tag はコマンド置換のサブシェルで走り EXIT trap を継承しないので、config の作成は親シェルの run_storybook で行う。 テスト: fake_server に AUTH_LOG(受信した Authorization ヘッダーの記録)を 追加し、config ファイルの権限が 600 であること・サーバーに Bearer トークン が届くこと・タグ解決が成功することを検証するケースを追加。
fdAT は小文字始まりで見た目は ancillary だが、png crate では IDAT と同じ データ chunk の専用経路に入り、最初の IDAT より前に現れると長さ・CRC を 見る前に UnexpectedRestartOfDataChunkSequence で致命になる。従来実装は 一般 ancillary として読み飛ばしていたため、細工 PNG が事前検証を通り、 ビルド作成後のアップロードで拒否され得た。 fcTL 分岐の前に fdAT 専用分岐を追加し、CRC や長さにかかわらず拒否する。 検証: CRC 正常・CRC 破壊・fcTL 後・短小ペイロードの 4 種の fdAT PNG を png 0.18.1 read_info と本実装の両方に与え、全件が同判定(拒否)である ことを確認。回帰テストに CRC 正常/破壊の両 fixture を追加。tests/run.sh 全件 PASS。
- screenshots: status GET は冪等なので、poll 中の 5xx は transport エラーと 同様に deadline まで再試行する。4xx は従来どおり恒久エラーとして即失敗 - lib: POLL_INTERVAL/TIMEOUT をテスト専用の VRT_TEST_* 環境変数で上書き 可能にし、poll の 502 再試行とタイムアウト(exit 2 + outputs)を e2e テストで検証できるようにした - storybook: wait:false でも CLI が非ゼロ終了した実行は CODE=2 にする。 JSON を出力して exit 0 の場合のみ成功扱い - README: CLI チェックサムの記述を「転送整合性の確認」に弱め、供給網固定は #2 で対応予定であることを明記 - tests: 502 再試行・poll タイムアウト・wait:false + CLI 失敗の 3 ケース追加。 fake server に FLAKY_POLL_CODE、fake CLI に終了コード注入口を追加
概要
README で先に定めた入出力設計に沿って、action を実装する。これまでこのリポジトリは README 1 ファイルのみで、
uses:に指定しても動作しなかった。実装方針
composite action として実装した(TypeScript ではない)。理由は、bundle した
dist/をコミットする運用を避け、変更がそのままレビューできる状態を保つため。GitHub のランナーにはcurl/jq/git/tarがあるのでシェルで完結する。action.ymlscripts/main.shscripts/lib.shscripts/validate.shscripts/collect-pngs.shscripts/screenshots.shscripts/storybook.shscreenshots: REST を直接叩く(ビルド作成 → PNG を 1 枚ずつ → finalize → ポーリング)。PNG 以外の混入・25 MiB 超・空ディレクトリはアップロード開始前に全件チェックして落とす。中途半端なビルドを作らないためstorybook: koyori-app/vrt の Release から CLI を取得(SHA256 チェックサム検証あり)して委譲するREADME からの意図的な変更(2 点)
1.
commitの既定値README は
GITHUB_SHAとしていたが、github.event.pull_request.head.shaを優先する実装にした。pull_requestイベントのGITHUB_SHAは main へのマージコミットで、GitHub 上に永続しない。VRT に記録された commit SHA は GitHub App がコミットステータスを貼る先になるため、PR ブランチに実在する head SHA でなければ貼れない。README の該当行も理由付きで更新した。2. input を 2 つ追加
cli-version(既定latest)— storybook モードで使う CLI のリリースタグapp-url(既定は空)—build-url組み立て用の Web UI ベース URL。空のときはurlから末尾の/apiを除いた値を使う。リバースプロキシ構成では API のベースがhttps://example.com/api、UI のベースがhttps://example.comになり得るためセキュリティ上の扱い
::add-mask::でマスク--config(mode 600 の一時ファイル)でヘッダーを渡し、psから見える argv にトークンを置かない。終了時に trap で削除set -xは使わないレビューで見てほしい点
実装後の検証で終了コードのバグを 1 件見つけて直した(2 コミット目)。
dieが終了コードを: "${CODE:=2}"で決めていたが、main.shが dispatch 前にCODE=0を初期化しているため代入が発火せず、ビルド作成後の致命的エラーでも exit 0 で終了していた。outputs にはresult=failed/exit-code=0という矛盾した値が書かれ、アップロード失敗や HTTP エラーでもワークフローが緑のまま通る状態だった。終了コードを明示的に代入する形へ修正し、3 経路すべてを実際に実行して確認した。
result=failed/exit-code=2)確認したこと
koalaman/shellcheck:stableで実行)。SC2034 の誤検知は、main.sh が全スクリプトを source して変数を共有する構造によるものなので、理由コメント付きのディレクティブで抑制したbash -n全 6 スクリプト構文エラーなし、action.ymlの YAML パース OK(inputs 12 / outputs 4)only-changed+screenshots/stats-json単独 /projectの形式 4 種 /token空 /url空 → すべて exit 1mobile/home.png→mobile/home(階層保持・拡張子除去)、PNG 以外の混入・空ディレクトリ・欠損ディレクトリ・25 MiB 超 → すべてアップロード前にエラー--json最終行のパース、wait: trueでの CLI 終了コード採用、wait: falseでの 0 固定、--json非対応の旧 CLI に対するエラーメッセージを確認未検証: 実 API を叩く経路(認証情報が無いため)。実際のリリースからの CLI ダウンロードも未検証。
依存
storybook モードには
cli-v0.1.0以降の vrt CLI が必要(--jsonフラグを使うため)。これは koyori-app/vrt#7 で実装済みで、マージ後にタグを切ると Release が作られる。screenshots モードはこの依存なしで動く。