Skip to content

fix(ci): CI が push されたブランチを検証するようにし、shell lint とテストを追加 - #28

Merged
ebkn merged 4 commits into
mainfrom
test/scripts
Aug 13, 2026
Merged

fix(ci): CI が push されたブランチを検証するようにし、shell lint とテストを追加#28
ebkn merged 4 commits into
mainfrom
test/scripts

Conversation

@ebkn

@ebkn ebkn commented Aug 13, 2026

Copy link
Copy Markdown
Owner

Summary

シェルスクリプトの仕様記載とテストカバレッジを調査した結果見つかった、CI の問題を修正する。

  1. セットアップ用 CI が push されたブランチを検証していなかった — macOS / Ubuntu / WSL の 3 ジョブは /main/ 固定の raw URL から bootstrap を curl し、その bootstrap が ref 指定なしで default branch を clone していた。つまり push したブランチが何であれ常に main を検証しており、未マージのコードでは原理的に落ちない。actions/checkout を使っている windows-setup.yml だけが実際の ref を検証していた。
  2. 静的解析とテストがどこからも実行されていなかったshellcheck は CLAUDE.md で必須とされ Brewfile にも入っているが、CI にもフックにも組み込まれていなかった。curl-guard.test.sh(このリポジトリで唯一の実質的なテスト)も手動実行のみ。
  3. curl-guard.test.sh が hermetic でなかった(CI に載せて初めて発覚) — 詳細は下記。

Changes

fix(ci): make setup workflows test the pushed branch

  • 3 つの setup workflow を actions/checkout → checkout を ~/dotfiles に symlink → DOTFILES_SKIP_UPDATE=1 で bootstrap 実行、という形に変更
  • bootstrap 3 本に DOTFILES_SKIP_UPDATE を追加。これはテスト用の足場ではなく、リビジョンを呼び出し側が決めるケース(作業中の clone からのローカル再実行など)で必要な機能。checkout が存在しない状態で指定した場合は silent clone ではなくエラー終了させ、指定ミスが main のインストールに化けないようにしている

feat(ci): add shell lint and hook test to CI

  • bin/lint-shell を追加:sh/bash に shellcheck -x -P SCRIPTDIR、shellcheck が解析できない zsh ファイル(SC1071)に zsh -n。計 37 ファイル
    • ロジックを workflow にインライン展開せずスクリプトとして持たせたのは、ローカルで再現できるようにするため。dotfiles で CI を常時見ることは少なく、push しないと動かないゲートは無視されやすい
    • 対象は shebang から発見するので、スクリプトを追加してもリストの更新は不要
  • .github/workflows/lint-and-test.yml を追加。lint と curl-guard.test.sh(41 ケース)を全 push で実行

fix(hooks): make curl-guard test read the repo's settings.json

CI に載せたことで発覚した、テスト自体の欠陥の修正。

  • フックは allow-list を $HOME/.claude/settings.json から読むため、テストは「実行マシンにインストールされている設定」を見ていた。セットアップ済みの Mac ではそのパスが本リポジトリへの symlink なので全て pass するが、CI にはファイルが存在せずフックは早期 exit し、全ケースが defer する
  • ALLOW 8 ケースが落ちたのは分かりやすい症状だが、本質的な問題は静かな方:DEFER 33 ケースは pass し続ける。「フックが黙っていた」は、そもそもロードされなかったフックの挙動と区別がつかない。セキュリティ側の半分が「正しく拒否した」と「一度も走らなかった」を判別できないテストは、無いより悪い
  • HOME をリポジトリの root/ に向け、テスト対象のツリー内の settings.json を読ませるよう修正。あわせて、ALLOW ケースが依存するドメインが fixture に実在することを事前アサートし、vacuous pass のモードでは green ではなく明示的に落ちるようにした

style(bin): make every shell script shellcheck-clean

  • common.sh / links.sh は source 専用で shebang がなく、shellcheck が SC2148 で受け付けないため # shellcheck shell=bash を付与
  • install_minimum_vim.sh の unquoted $HOME(SC2086)を修正

Concerns

  • setup 系 3 ジョブ(build)は失敗しているが、この PR とは無関係の既存の失敗。 brew bundle が tmux と gnupg のインストールに失敗する(unknown install step: run)。main の 2026-08-10 の run でも全く同じ 2 つの失敗が出ており、本 PR は Brewfile を触っていない。別途対応が必要。なお Windows ジョブは 6 時間でタイムアウト cancel されるのも既存の挙動
  • root/.codex/rules/default.rules.test.sh は意図的に CI に入れていない。 codex CLI が必要で、runner には存在しないため exit 0 の "skipping" になる。何も証明しない永続 green は、まさに今回 curl-guard のテストで踏んだ vacuous pass と同じ問題なので、ローカル実行のままにしている(CLAUDE.md に明記)
  • actionlint がローカル環境に入っていないため、workflow YAML 自体は未 lint

Verification

  • lint / test ジョブともに CI で green
  • bin/lint-shell → shellcheck 21 ファイル / zsh -n 16 ファイル いずれも clean
  • curl-guard.test.sh → 41 passed, 0 failed。HOME を存在しないパスに向けても pass することで hermetic 化を確認
  • DOTFILES_SKIP_UPDATE はスタブした ~/dotfiles に対して両パスを確認:skip パスは git に触れずに対象を exec、checkout 不在パスは明示メッセージ付きで exit 1

ebkn added 4 commits August 13, 2026 17:32
Preparation for putting shellcheck in CI. common.sh and links.sh are
sourced rather than executed, so they have no shebang and shellcheck
refuses them outright with SC2148 — a `shell=bash` directive is the only
way to get them checked at all. install_minimum_vim.sh had unquoted
$HOME expansions (SC2086).

No behavior change; the quoting fix only matters if $HOME contains
whitespace.
shellcheck was mandated by CLAUDE.md but never enforced anywhere, and
curl-guard.test.sh — the only real behavioral test in the repo, guarding
which curl invocations get auto-approved — ran only when someone
remembered to. Both now gate every push.

bin/lint-shell holds the logic instead of the workflow inlining it, so
the check is reproducible locally; in a dotfiles repo you rarely watch
CI, and a gate you can only trigger by pushing gets ignored. It
discovers targets by shebang so new scripts are covered without editing
a list, and runs zsh -n over the zsh files that shellcheck cannot parse
(SC1071) — 36 files in total.

default.rules.test.sh is deliberately excluded: it needs the codex CLI
and exits 0 with "skipping" when absent, which would be a permanently
green check proving nothing.
The macOS/Ubuntu/WSL jobs curled the bootstrap from a hardcoded /main/
raw URL, and the bootstrap then git-cloned the default branch with no
ref pin. So every run installed main regardless of which branch was
pushed: the checks could not fail on unmerged code and only went red
after a bad commit had already landed. Only windows-setup.yml, which
uses actions/checkout, was testing the actual ref.

The workflows now check out, symlink the checkout to ~/dotfiles, and run
the bootstrap with DOTFILES_SKIP_UPDATE=1.

DOTFILES_SKIP_UPDATE is a real feature, not test scaffolding: it is
needed by any caller that decides the revision itself, including a local
re-run from a work-in-progress clone. Setting it with no checkout
present is an error rather than a silent clone, so a typo cannot quietly
fall back to installing main.

CLAUDE.md documents both this and the new lint-shell entrypoint.
The hook resolves its allow-list from $HOME/.claude/settings.json, so
the test was reading whatever is installed on the machine. On a
provisioned Mac that path is symlinked into this repo and everything
passed; in CI the file is absent, the hook exits early, and every case
defers.

That failed the 8 ALLOW cases loudly, but the real problem is quiet: all
33 DEFER cases still passed, because "the hook stayed silent" is exactly
what a hook that never loaded does. A suite whose security half cannot
distinguish "correctly refused" from "never ran" is worse than no suite.

Point HOME at root/ so the hook reads the settings.json in the tree under
test, and assert up front that the fixture actually contains the domains
the ALLOW cases depend on, so the vacuous-pass mode fails loudly instead
of reporting green.
@ebkn
ebkn merged commit c09d8e1 into main Aug 13, 2026
3 of 7 checks passed
@ebkn
ebkn deleted the test/scripts branch August 13, 2026 13:26
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.

1 participant