From b292ac27c95470f9e28a2b774485904410e47121 Mon Sep 17 00:00:00 2001 From: Yoshihiro Misawa Date: Wed, 23 Sep 2026 19:40:10 +0900 Subject: [PATCH 1/2] Retire CodeRabbit: keep its review contract and checks CodeRabbit is no longer used. Removing .coderabbit.yaml outright would have dropped two guard layers AGENTS.md relies on, so both are kept: - Its review contract (the path instructions) moves to a new Review checklist in AGENTS.md, in English. It also covers the rules added since: clean verdicts only from a NUL-terminated reply (ADR-0007) and no source error other than io.EOF treated as end of input. - Its workflow checks move to the required lint job: actionlint (pinned, also run by make lint) and zizmor (action pinned by SHA). The gating zizmor run reports as annotations and fails the job; a separate zizmor-sarif job, the only one with security-events: write, uploads the same audit to code scanning, which cannot gate because zizmor exits 0 with SARIF output. gitleaks is covered by GitHub secret scanning with push protection; yamllint and markdownlint are dropped. The CodeRabbit review step becomes an independent review against the checklist by someone other than the author (the maintainer, a fresh reviewer agent or Codex adversarial-review). The review must cover the merged commit, which the PR records, and every finding must be fixed or answered before merging. AGENTS.md, CONTRIBUTING.md, CLAUDE.md, the pr-flow skill and the PR template say so consistently. ADR-0002 keeps its original text with a dated note. zizmor's dependabot-cooldown audit flagged the Dependabot config, so every ecosystem now waits 7 days before proposing a new version; security updates are not delayed. --- .claude/skills/pr-flow/SKILL.md | 36 ++++--- .coderabbit.yaml | 115 ----------------------- .github/PULL_REQUEST_TEMPLATE.md | 1 + .github/dependabot.yml | 11 +++ .github/workflows/ci.yaml | 34 +++++++ AGENTS.md | 78 ++++++++++++--- CHANGELOG.md | 10 ++ CLAUDE.md | 6 +- CONTRIBUTING.md | 18 ++-- Makefile | 5 +- README.ja.md | 2 +- README.md | 2 +- docs/adr/0002-fail-closed-error-model.md | 4 + 13 files changed, 167 insertions(+), 155 deletions(-) delete mode 100644 .coderabbit.yaml diff --git a/.claude/skills/pr-flow/SKILL.md b/.claude/skills/pr-flow/SKILL.md index 5f21392..2d7a261 100644 --- a/.claude/skills/pr-flow/SKILL.md +++ b/.claude/skills/pr-flow/SKILL.md @@ -1,13 +1,14 @@ --- name: pr-flow -description: Land current changes on main via the required PR workflow of this repository — feature branch, signed commits, required checks, CodeRabbit review handling, merge-commit merge. Use when asked to open a PR, ship/land changes, or merge work into main. +description: Land current changes on main via the required PR workflow of this repository — feature branch, signed commits, required checks, independent review, merge-commit merge. Use when asked to open a PR, ship/land changes, or merge work into main. --- # PR flow for PyYoshi/go-clamav -`main` only accepts merge commits from PRs with green required checks and a -non-blocking CodeRabbit review. Follow the steps in order; never bypass a -step with `--no-verify`, force pushes or `gh pr merge --admin`. +`main` only accepts signed merge commits from PRs with green required +checks, and every PR gets an independent review before it is merged. Follow +the steps in order; never bypass a step with `--no-verify`, force pushes or +`gh pr merge --admin`. ## 1. Before committing @@ -43,17 +44,22 @@ gh pr checks --watch Required: `unit`, `lint`, `integration (1.4)`, `integration (1.5)`. Fix failures and push; never merge around them. -## 5. Handle CodeRabbit - -CodeRabbit reviews in Japanese with request-changes enabled; its -CHANGES_REQUESTED review blocks the merge (mergeStateStatus=BLOCKED). - -- Address valid findings, push the fixes. -- For findings you reject, reply on the finding's thread with the concrete - reason (never resolve silently). -- When every finding is handled, comment on the PR: - `@coderabbitai resolve` — CodeRabbit re-checks and posts an approving - review; the merge state becomes CLEAN. +## 5. Independent review + +No bot review gates the merge, so this step is where the PR gets reviewed. +Have the diff reviewed against the Review checklist in AGENTS.md by a +reviewer that is not the session that wrote it: the maintainer, or a +separate review run such as a fresh reviewer agent or Codex +`adversarial-review` (pass the checklist as its focus text). + +- Address valid findings and push the fixes; the required checks rerun, + and the fixes go back to the reviewer — the review must cover the commit + that is merged. +- For findings you reject, reply in the PR with the concrete reason (never + drop one silently). +- Record the reviewed commit SHA in the PR. Merge only once every finding, + whatever its severity, is fixed or answered, then tick the review item + in the PR checklist. ## 6. Merge and clean up diff --git a/.coderabbit.yaml b/.coderabbit.yaml deleted file mode 100644 index 19da465..0000000 --- a/.coderabbit.yaml +++ /dev/null @@ -1,115 +0,0 @@ -# yaml-language-server: $schema=https://storage.googleapis.com/coderabbit_public_assets/schema.v2.json -# -# CodeRabbit configuration for go-clamav. -# -# This library is a fail-closed security control; the path instructions -# below encode its review contract so automated reviews look for the same -# invariants the human reviews enforce. - -language: "ja-JP" - -reviews: - profile: "assertive" - request_changes_workflow: true - high_level_summary: true - poem: false - - auto_review: - enabled: true - drafts: false - base_branches: - - "main" - - path_filters: - - "!docker/clamd/db/**" # hex-encoded EICAR signature database, not reviewable content - - "!docker/run/**" # runtime bind-mount directory (.gitkeep only) - - "!LICENSE" - - path_instructions: - - path: "**/*.go" - instructions: | - このライブラリはfail-closedなセキュリティ制御です。以下の契約違反を最優先で指摘してください: - - err != nil のときにゼロ値以外の ScanResult が返り得る経路(ゼロ値 = VerdictUnknown が契約) - - 解釈できない clamd 応答が VerdictClean に分類され得る変更(fail-open化) - - 応答読み取り上限・I/Oデッドライン・クライアント側サイズ上限の緩和や削除 - - 部分ストリームが完全なストリームとして clamd に提示され得る変更(終端チャンクの扱い) - 加えて: 標準ライブラリ以外の依存追加(このライブラリは依存ゼロが方針)、context を無視した - ブロッキングI/O、goroutineリーク、エラーメッセージへのスキャン内容の混入を指摘してください。 - godoc とコードコメントは英語で統一します。 - ユーザーから見える振る舞いの変更に CHANGELOG.md の [Unreleased] エントリが - 伴っていない場合も指摘してください。 - - path: "go.mod" - instructions: | - このライブラリは依存ゼロ(標準ライブラリのみ)が方針です。 - require の追加は重大な方針逸脱として指摘してください。 - - path: "internal/proto/**" - instructions: | - clamd ワイヤプロトコル層です。応答パーサは接頭辞非依存・サフィックス駆動で、 - 分類優先順位は FOUND > ERROR > OK、未知の応答は必ず OutcomeUnknown にします。 - この優先順位や未知応答の扱いを緩める変更(fail-open方向)は重大な欠陥として扱ってください。 - - path: "internal/clamdtest/**" - instructions: | - テスト用フェイク clamd と EICAR ヘルパーの実体です。EICAR テスト文字列の完全形 - (68バイト)をリポジトリに持ち込む変更は拒否してください(分割定数・hex 表記を維持。 - チェックアウトが常駐AVに隔離される事故の防止)。 - - path: "**/*_test.go" - instructions: | - タイミング依存で flaky になり得るテスト、-race で壊れる並行アクセスを指摘してください。 - EICAR テスト文字列の完全形(68バイト)をリポジトリに持ち込む変更は拒否してください - (分割定数・hex 表記を維持。チェックアウトが常駐AVに隔離される事故の防止)。 - - path: ".github/**" - instructions: | - GitHub Actions のバージョン固定と permissions の最小化を確認してください。 - CI ジョブ名(unit / lint / integration (1.4) / integration (1.5))を変更する場合は - branch ruleset の必須ステータスチェック名との同期が必要である旨を指摘してください。 - - path: "docker/**" - instructions: | - 統合テスト専用の環境定義です。clamd.conf の制限値(StreamMaxLength 等)の変更が - integration_test.go の期待値と整合しているか確認してください。 - - path: "README.md" - instructions: | - README.md(英語)と README.ja.md(日本語)は内容を同期させる方針です。 - 片方だけが変更されている場合は乖離として指摘してください。 - - path: "README.ja.md" - instructions: | - README.ja.md(日本語)と README.md(英語)は内容を同期させる方針です。 - 片方だけが変更されている場合は乖離として指摘してください。 - - path: "AGENTS.md" - instructions: | - 人間・AI共通の開発規約の正本です。不変条件・Definition of Done・設計ゲート・ - git規約を削除・緩和する変更は重大な指摘として扱ってください。 - CLAUDE.md(@AGENTS.md import)および CONTRIBUTING.md との整合も確認してください。 - - path: ".claude/**" - instructions: | - Claude Code 用の開発ハーネス(hooks・skills・settings)です。ガードの無効化・ - 緩和 — exit 0 への変更、検出パターンやブロック対象の削除、Stop hook の除去、 - hooks 配線の削除 — は重大な指摘として扱ってください。 - - path: "githooks/**" - instructions: | - 全コントリビューター共通の git フックです。Co-Authored-By 拒否・EICAR 検査の - 削除や緩和は重大な指摘として扱ってください。 - - path: "scripts/**" - instructions: | - git フック・CI・Claude Code hooks が共有するガードロジックです。EICAR パターンは - 実行時に hex から組み立てる方式を維持してください(平文定数化は拒否)。検査の - 削除・緩和・fail-open 化は重大な指摘として扱ってください。 - - tools: - github-checks: - enabled: true - timeout_ms: 300000 # the ClamAV integration matrix takes a few minutes - golangci-lint: - enabled: true # uses this repository's .golangci.yml (security-heavy config) - gitleaks: - enabled: true - actionlint: - enabled: true - zizmor: - enabled: true # GitHub Actions security audit - yamllint: - enabled: true - markdownlint: - enabled: true - -chat: - auto_reply: true diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index db6d7b2..f9bc2d9 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -10,3 +10,4 @@ - [ ] `README.md` and `README.ja.md` updated together, or the change touches neither - [ ] `CHANGELOG.md` updated under `[Unreleased]` for user-visible changes - [ ] No new dependencies, no assembled EICAR string, no weakened guards; design changes reference an ADR +- [ ] Reviewed against the AGENTS.md Review checklist by someone other than the author, covering the merged commit (recorded in the PR); every finding fixed or answered diff --git a/.github/dependabot.yml b/.github/dependabot.yml index d9e107f..3fa111d 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -1,17 +1,26 @@ # Dependabot: weekly checks for the Go module (currently stdlib-only, but # this also covers future additions), GitHub Actions pins, and the ClamAV # image references in the compose file. +# +# Every ecosystem waits 7 days before proposing a newly published version, +# so a compromised release has time to be caught upstream before it lands +# here (zizmor's dependabot-cooldown audit). Cooldown applies to version +# updates only; security updates are not delayed. version: 2 updates: - package-ecosystem: gomod directory: "/" schedule: interval: weekly + cooldown: + default-days: 7 - package-ecosystem: github-actions directory: "/" schedule: interval: weekly + cooldown: + default-days: 7 # Note: the clamd image is referenced as ${CLAMAV_IMAGE:-clamav/clamav:1.4}. # If Dependabot cannot parse the env-default form, remove this entry — @@ -20,3 +29,5 @@ updates: directory: "/docker" schedule: interval: weekly + cooldown: + default-days: 7 diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 92b0e22..0193bdc 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -56,6 +56,40 @@ jobs: run: | go install golang.org/x/vuln/cmd/govulncheck@v1.7.0 govulncheck ./... + # Workflow checks that previously ran only inside CodeRabbit. They are + # steps of this required job, so a finding blocks the merge. + - name: actionlint (workflow syntax, expressions, shellcheck) + run: | + go install github.com/rhysd/actionlint/cmd/actionlint@v1.7.12 + actionlint + - name: zizmor (GitHub Actions security audit) + uses: zizmorcore/zizmor-action@cc914d7f3750a2d13d75c7f184a1060aa0e9d482 # v0.6.4 + with: + version: 1.30.1 + # This is the gating run: report in the job log and as PR + # annotations and fail the job on findings. The zizmor-sarif job + # uploads the same audit to code scanning, which cannot gate: + # with SARIF output zizmor exits 0 even when it finds something. + advanced-security: false + annotations: true + + # Uploads the zizmor audit to code scanning (Security tab) for alert + # tracking. The only job with security-events: write, so the lint job + # stays read-only; merge gating is the zizmor step in lint. + zizmor-sarif: + runs-on: ubuntu-latest + permissions: + contents: read # check out the workflows to audit + security-events: write # upload the SARIF results to code scanning + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + - name: zizmor (upload to code scanning) + uses: zizmorcore/zizmor-action@cc914d7f3750a2d13d75c7f184a1060aa0e9d482 # v0.6.4 + with: + version: 1.30.1 + advanced-security: true # Required: pinned ClamAV versions. 1.4 is the LTS default (EOL # 2027-08-15), 1.5 the current regular release. 1.0 was dropped at its diff --git a/AGENTS.md b/AGENTS.md index 87fcde7..8d47da6 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -38,9 +38,9 @@ Changing any of these is a defect, not a refactor: scanned data. These invariants are enforced in layers: tests, CI, git hooks, Claude Code -hooks, CodeRabbit path instructions and GitHub rulesets. **Weakening a -guard layer (disabling a hook, deleting a check, softening an instruction) -is itself a critical defect.** +hooks, the independent review against the Review checklist below, and +GitHub rulesets. **Weakening a guard layer (disabling a hook, deleting a +check, softening an instruction) is itself a critical defect.** ## Definition of Done @@ -72,6 +72,63 @@ Humans decide design; agents implement it. **Stop and ask the maintainer Accepted designs are recorded as `docs/adr/NNNN-title.md` (start from `docs/adr/template.md`). +## Review checklist + +Every PR is reviewed against this list before it is merged, by a reviewer +other than its author: the maintainer, or a separate review run such as a +fresh reviewer agent or Codex `adversarial-review`. Valid findings are +fixed; a rejected finding is answered in the PR with the reason. No bot +review gates the merge, so this review is the only one a PR gets: + +- it must cover the commit that is merged — fixes pushed after a review + go back to the reviewer, and the PR records the reviewed commit; +- a PR is merged only once every finding, whatever its severity, is fixed + or answered with a reason. + +Critical — report these first: + +- A path that returns a non-zero `ScanResult` together with `err != nil` + (invariant 1). +- A change that lets an unclassifiable clamd reply become `VerdictClean`, + or relaxes the reply classification in `internal/proto`: the + FOUND > ERROR > OK priority, the prefix-agnostic suffix matching of + FOUND and ERROR, the exact OK allowlist (ADR-0005), clean verdicts only + from a NUL-terminated reply (ADR-0007), or unknown replies mapping to + `OutcomeUnknown` (invariant 2). +- Relaxed or removed reply-read bounds, I/O deadlines or client-side size + limit (invariant 3). +- A change that could present a partial stream to clamd as complete: the + INSTREAM terminator handling, including any source error other than + `io.EOF` treated as end of input (invariant 4). +- A `require` in `go.mod` or any other non-stdlib dependency (invariant 5). +- The complete 68-byte EICAR string in the repository, or the + hex-assembled EICAR pattern in `scripts/` turned into a plaintext + constant (invariant 6). +- Scanned content in error messages or logs (invariant 7). +- A weakened guard: an invariant, the Definition of Done, the design gate + or the git rules in this file deleted or relaxed; a hook in `.claude/` + or `githooks/` disabled or relaxed (changed to exit 0, detection patterns + or block targets removed, wiring or the Stop hook removed, including the + `Co-Authored-By` and EICAR checks); a check in `scripts/` removed, + relaxed or made fail-open. + +Also check: + +- Blocking I/O that ignores the context, and goroutine leaks. +- Tests that are timing-dependent or race under `-race`. +- godoc and comments in English; a `CHANGELOG.md` `[Unreleased]` entry for + user-visible behavior changes; `README.md` and `README.ja.md` changed + together. +- Changes to this file kept consistent with `CLAUDE.md` (which imports + it) and `CONTRIBUTING.md`. +- `.github/`: actions pinned to commit SHAs and minimal `permissions` + (actionlint and zizmor also check workflows in the `lint` job); a + renamed CI job (`unit`, `lint`, `integration (1.4)`, + `integration (1.5)`) renamed in the branch ruleset's required checks + too. +- `docker/`: clamd.conf limits such as `StreamMaxLength` consistent with + the expectations in `integration_test.go`. + ## Git rules - **Never** add `Co-Authored-By` or other attribution trailers to commits. @@ -79,26 +136,23 @@ Accepted designs are recorded as `docs/adr/NNNN-title.md` verify with `git log --format='%h %G?'` (expect `G`). - Never push to `main`. Work on a feature branch, open a PR, wait for the four required checks (`unit`, `lint`, `integration (1.4)`, - `integration (1.5)`) and CodeRabbit, then merge with a merge commit - (`gh pr merge --merge --delete-branch`). Squash and rebase merges are - disabled to preserve signatures. + `integration (1.5)`) and the independent review (see Review checklist), + then merge with a merge commit (`gh pr merge --merge --delete-branch`). + Squash and rebase merges are disabled to preserve signatures. - Never use `--no-verify`, `--no-gpg-sign`, force pushes, `gh pr merge --admin`, or `core.hooksPath` overrides. - Run `make setup` once per clone to enable the repository git hooks. -See CONTRIBUTING.md for the full pull-request and CodeRabbit workflow. +See CONTRIBUTING.md for the full pull-request and review workflow. ## Language policy - Everything that becomes part of the repository or its history is written in **English**: code, comments, godoc, documentation, ADRs, CHANGELOG entries, commit messages, and pull-request titles and descriptions. -- Exceptions: `README.ja.md` (the Japanese mirror of `README.md`) and the - Japanese instruction text in `.coderabbit.yaml` (review output is - deliberately ja-JP). +- Exception: `README.ja.md` (the Japanese mirror of `README.md`). - Interaction language is not fixed: conversations with AI assistants and - review-thread discussions follow the participants' preference (e.g. - CodeRabbit reviews and replies to it are typically in Japanese). + review discussions follow the participants' preference. ## Commands diff --git a/CHANGELOG.md b/CHANGELOG.md index 7fc0d5b..0eaafb0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,16 @@ project adheres to [Semantic Versioning](https://semver.org/). govulncheck is pinned to a release version in CI and the Makefile instead of `@latest` (the vulnerability database is still fetched live at scan time). +- CodeRabbit is retired. Its review contract now lives in the Review + checklist in AGENTS.md, and every PR gets an independent review against + it before merging. The workflow checks CodeRabbit used to run now run in + the required `lint` job: actionlint (also in `make lint`) and zizmor. + A separate `zizmor-sarif` job, the only one with `security-events: + write`, also uploads the zizmor audit to code scanning; it does not + gate, since zizmor exits 0 with SARIF output. Secret scanning remains + covered by GitHub's secret scanning with push protection; YAML and + Markdown style linting is dropped. Dependabot now waits 7 days before + proposing a new version (security updates are not delayed). - Reply classification is stricter: a clean verdict is now produced only by the exact reply lines `stream: OK` or `OK` (ADR-0005). Previously any `: OK` whose prefix contained "stream" diff --git a/CLAUDE.md b/CLAUDE.md index ead45a2..c281d0e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -6,6 +6,6 @@ rules in-session; the Stop hook blocks once when Go sources changed but `make verify` has not passed against them. Do not work around a blocked action — fix the cause or ask the user. -- Skills: `/pr-flow` (feature branch → PR → CodeRabbit → merge commit) and - `/release` (CHANGELOG cut + signed tag + GitHub release; explicit - invocation only). +- Skills: `/pr-flow` (feature branch → PR → independent review → merge + commit) and `/release` (CHANGELOG cut + signed tag + GitHub release; + explicit invocation only). diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index b983f84..930f19e 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -64,15 +64,19 @@ through pull requests. (signing is automatic via repo config; the commit-msg hook enforces the trailer rule). 3. Push the branch and open a PR (`gh pr create`); fill in the template - checklist — it mirrors the Definition of Done in AGENTS.md. + checklist — it mirrors the Definition of Done in AGENTS.md, plus the + review in step 5 (tick that item once the review is done). 4. Wait for the required checks: `unit`, `lint`, `integration (1.4)`, `integration (1.5)`. -5. CodeRabbit reviews every PR (in Japanese, with request-changes enabled — - its CHANGES_REQUESTED state blocks merging): - - address findings and push fixes; - - when rejecting a finding, reply on its thread with the reason; - - once every finding is handled, comment `@coderabbitai resolve` on the - PR — CodeRabbit re-checks and posts an approving review. +5. Get an independent review: before merging, a reviewer other than the + author — the maintainer, or a separate review run such as a fresh + reviewer agent or Codex `adversarial-review` — reviews the diff against + the Review checklist in AGENTS.md: + - address valid findings and push fixes, then have the fixes reviewed + too — the review must cover the commit that is merged; + - when rejecting a finding, reply in the PR with the reason; + - record the reviewed commit in the PR, and merge only once every + finding, whatever its severity, is fixed or answered. 6. Merge with a merge commit: `gh pr merge --merge --delete-branch` (squash/rebase merges are disabled so commit signatures survive). diff --git a/Makefile b/Makefile index 8aa9fe2..6b76399 100644 --- a/Makefile +++ b/Makefile @@ -6,6 +6,8 @@ GOLANGCI_LINT ?= $(GO) run github.com/golangci/golangci-lint/v2/cmd/golangci-lin GOVULNCHECK_VERSION ?= v1.7.0 +ACTIONLINT_VERSION ?= v1.7.12 + .PHONY: test test: ## Unit tests (no Docker required) $(GO) vet ./... @@ -54,9 +56,10 @@ setup: ## One-time developer setup: enable git hooks, check tooling @command -v jq >/dev/null 2>&1 || echo "WARNING: jq not found; Claude Code guard hooks are inert without it." .PHONY: lint -lint: ## Static analysis (golangci-lint + govulncheck) +lint: ## Static analysis (golangci-lint + govulncheck + actionlint) $(GOLANGCI_LINT) run $(GO) run golang.org/x/vuln/cmd/govulncheck@$(GOVULNCHECK_VERSION) ./... + $(GO) run github.com/rhysd/actionlint/cmd/actionlint@$(ACTIONLINT_VERSION) .PHONY: format format: ## Format code with gofumpt + gci (via golangci-lint fmt) diff --git a/README.ja.md b/README.ja.md index dd031e9..7b597b4 100644 --- a/README.ja.md +++ b/README.ja.md @@ -154,7 +154,7 @@ clamdのプロトコルは**認証なしの平文**です。アドレスはセ make test # ユニットテスト+レース検出(Docker不要) make integration # Docker上のclamdを起動して統合テストを実行 make fuzz # 応答パーサの短時間ファジング -make lint # golangci-lint(セキュリティ強め設定)+ govulncheck +make lint # golangci-lint(セキュリティ強め設定)+ govulncheck + actionlint make format # gofumpt + gci による整形(golangci-lint fmt経由) ``` diff --git a/README.md b/README.md index f86aad5..e4fabfd 100644 --- a/README.md +++ b/README.md @@ -157,7 +157,7 @@ freshness, overload behavior) is covered in make test # unit tests + race detector (no Docker needed) make integration # starts dockerized clamd, runs integration suite make fuzz # short fuzzing pass over the reply parser -make lint # golangci-lint (security-heavy config) + govulncheck +make lint # golangci-lint (security-heavy config) + govulncheck + actionlint make format # gofumpt + gci formatting (via golangci-lint fmt) ``` diff --git a/docs/adr/0002-fail-closed-error-model.md b/docs/adr/0002-fail-closed-error-model.md index e02ee7b..7547447 100644 --- a/docs/adr/0002-fail-closed-error-model.md +++ b/docs/adr/0002-fail-closed-error-model.md @@ -54,3 +54,7 @@ Every error path is fail-closed, mechanically: - Tests must assert the zero result on every error path; the review contract (.coderabbit.yaml, AGENTS.md) treats any deviation as a critical defect. + + *Note (2026-09-23): CodeRabbit was retired and `.coderabbit.yaml` + removed; its review contract now lives in the Review checklist in + AGENTS.md. The original text is left as written.* From df705c80f24d22ab8a5a2788b0d664bae8ab07a6 Mon Sep 17 00:00:00 2001 From: Yoshihiro Misawa Date: Wed, 23 Sep 2026 19:44:35 +0900 Subject: [PATCH 2/2] Keep generic secret detection: run gitleaks in the lint job The independent review of b292ac2 found that dropping CodeRabbit's gitleaks check leaves a gap: GitHub secret scanning covers provider token formats, and its non-provider patterns are disabled on this repository, so generic high-entropy credentials were no longer caught. Run gitleaks (pinned v8.30.1, installed with go install like actionlint) in the required lint job over the whole git history, so a secret that is committed and removed again within a PR is still found; the lint job checks out full history for it. make lint runs the same scan. The current history scans clean, and a synthetic generic API key is detected by the generic-api-key rule. The CHANGELOG no longer claims GitHub's scanning replaces gitleaks, and the Review checklist asks reviewers to look for credentials in the diff. --- .github/workflows/ci.yaml | 11 +++++++++-- AGENTS.md | 2 ++ CHANGELOG.md | 14 +++++++------- Makefile | 5 ++++- README.ja.md | 2 +- README.md | 2 +- 6 files changed, 24 insertions(+), 12 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 0193bdc..0222e23 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -36,6 +36,7 @@ jobs: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: persist-credentials: false + fetch-depth: 0 # full history: gitleaks scans every commit - name: EICAR plaintext guard run: bash ./scripts/check-eicar.sh tracked - name: Zero-dependency policy (ADR-0003) @@ -56,8 +57,14 @@ jobs: run: | go install golang.org/x/vuln/cmd/govulncheck@v1.7.0 govulncheck ./... - # Workflow checks that previously ran only inside CodeRabbit. They are - # steps of this required job, so a finding blocks the merge. + # Checks that previously ran only inside CodeRabbit. They are steps of + # this required job, so a finding blocks the merge. + - name: gitleaks (secrets in any commit) + # Scans the whole history, so a secret committed and later removed + # in the same PR is still caught; findings are printed redacted. + run: | + go install github.com/zricethezav/gitleaks/v8@v8.30.1 + gitleaks git --redact --no-banner --verbose - name: actionlint (workflow syntax, expressions, shellcheck) run: | go install github.com/rhysd/actionlint/cmd/actionlint@v1.7.12 diff --git a/AGENTS.md b/AGENTS.md index 8d47da6..8f73242 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -114,6 +114,8 @@ Critical — report these first: Also check: +- Credentials or tokens in the diff (gitleaks also scans the whole + history in the `lint` job). - Blocking I/O that ignores the context, and goroutine leaks. - Tests that are timing-dependent or race under `-race`. - godoc and comments in English; a `CHANGELOG.md` `[Unreleased]` entry for diff --git a/CHANGELOG.md b/CHANGELOG.md index 0eaafb0..b1003f3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -38,13 +38,13 @@ project adheres to [Semantic Versioning](https://semver.org/). - CodeRabbit is retired. Its review contract now lives in the Review checklist in AGENTS.md, and every PR gets an independent review against it before merging. The workflow checks CodeRabbit used to run now run in - the required `lint` job: actionlint (also in `make lint`) and zizmor. - A separate `zizmor-sarif` job, the only one with `security-events: - write`, also uploads the zizmor audit to code scanning; it does not - gate, since zizmor exits 0 with SARIF output. Secret scanning remains - covered by GitHub's secret scanning with push protection; YAML and - Markdown style linting is dropped. Dependabot now waits 7 days before - proposing a new version (security updates are not delayed). + the required `lint` job: gitleaks over the whole git history, + actionlint (both also in `make lint`) and zizmor. A separate + `zizmor-sarif` job, the only one with `security-events: write`, also + uploads the zizmor audit to code scanning; it does not gate, since + zizmor exits 0 with SARIF output. YAML and Markdown style linting is + dropped. Dependabot now waits 7 days before proposing a new version + (security updates are not delayed). - Reply classification is stricter: a clean verdict is now produced only by the exact reply lines `stream: OK` or `OK` (ADR-0005). Previously any `: OK` whose prefix contained "stream" diff --git a/Makefile b/Makefile index 6b76399..d127ffc 100644 --- a/Makefile +++ b/Makefile @@ -8,6 +8,8 @@ GOVULNCHECK_VERSION ?= v1.7.0 ACTIONLINT_VERSION ?= v1.7.12 +GITLEAKS_VERSION ?= v8.30.1 + .PHONY: test test: ## Unit tests (no Docker required) $(GO) vet ./... @@ -56,10 +58,11 @@ setup: ## One-time developer setup: enable git hooks, check tooling @command -v jq >/dev/null 2>&1 || echo "WARNING: jq not found; Claude Code guard hooks are inert without it." .PHONY: lint -lint: ## Static analysis (golangci-lint + govulncheck + actionlint) +lint: ## Static analysis (golangci-lint + govulncheck + actionlint + gitleaks) $(GOLANGCI_LINT) run $(GO) run golang.org/x/vuln/cmd/govulncheck@$(GOVULNCHECK_VERSION) ./... $(GO) run github.com/rhysd/actionlint/cmd/actionlint@$(ACTIONLINT_VERSION) + $(GO) run github.com/zricethezav/gitleaks/v8@$(GITLEAKS_VERSION) git --redact --no-banner .PHONY: format format: ## Format code with gofumpt + gci (via golangci-lint fmt) diff --git a/README.ja.md b/README.ja.md index 7b597b4..1867f03 100644 --- a/README.ja.md +++ b/README.ja.md @@ -154,7 +154,7 @@ clamdのプロトコルは**認証なしの平文**です。アドレスはセ make test # ユニットテスト+レース検出(Docker不要) make integration # Docker上のclamdを起動して統合テストを実行 make fuzz # 応答パーサの短時間ファジング -make lint # golangci-lint(セキュリティ強め設定)+ govulncheck + actionlint +make lint # golangci-lint(セキュリティ強め設定)+ govulncheck + actionlint + gitleaks make format # gofumpt + gci による整形(golangci-lint fmt経由) ``` diff --git a/README.md b/README.md index e4fabfd..a0bbf78 100644 --- a/README.md +++ b/README.md @@ -157,7 +157,7 @@ freshness, overload behavior) is covered in make test # unit tests + race detector (no Docker needed) make integration # starts dockerized clamd, runs integration suite make fuzz # short fuzzing pass over the reply parser -make lint # golangci-lint (security-heavy config) + govulncheck + actionlint +make lint # golangci-lint (security-heavy config) + govulncheck + actionlint + gitleaks make format # gofumpt + gci formatting (via golangci-lint fmt) ```