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..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,6 +57,46 @@ jobs: run: | go install golang.org/x/vuln/cmd/govulncheck@v1.7.0 govulncheck ./... + # 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 + 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..8f73242 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,65 @@ 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: + +- 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 + 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 +138,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..b1003f3 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: 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/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..d127ffc 100644 --- a/Makefile +++ b/Makefile @@ -6,6 +6,10 @@ GOLANGCI_LINT ?= $(GO) run github.com/golangci/golangci-lint/v2/cmd/golangci-lin 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 ./... @@ -54,9 +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) +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 dd031e9..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 +make lint # golangci-lint(セキュリティ強め設定)+ govulncheck + actionlint + gitleaks make format # gofumpt + gci による整形(golangci-lint fmt経由) ``` diff --git a/README.md b/README.md index f86aad5..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 +make lint # golangci-lint (security-heavy config) + govulncheck + actionlint + gitleaks 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.*