Skip to content

feat: audit-actions のスコープ明文化 & ツールハンドオフ追加#27

Merged
yk-lab merged 4 commits intomainfrom
fix/audit-actions-scope
Mar 28, 2026
Merged

feat: audit-actions のスコープ明文化 & ツールハンドオフ追加#27
yk-lab merged 4 commits intomainfrom
fix/audit-actions-scope

Conversation

@yk-lab
Copy link
Copy Markdown
Contributor

@yk-lab yk-lab commented Mar 28, 2026

Summary

  • audit-actions のスコープを Core(Action 関連セキュリティ)Reference(ワークフロー全般) に明確に分類
  • Reference チェックには zizmor/ghalint の自動検出ルール名を明記
  • 存在しない harden-workflows スキルへの参照を汎用表現に修正
  • スキルレビュー指摘を全件対応

Changes

SKILL.md

  • Scope セクション追加(Core の守備範囲を明示、pull_request_target 追加)
  • Workflow-wide security tooling セクション追加(actionlint/ghalint/zizmor 推奨)
  • トリガーフレーズ: "harden my CI pipelines" → "supply chain security audit"
  • Output format: severity ヘッダを Title Case に統一

security-checklist.md

Test plan

  • ./scripts/validate.sh 全チェックパス
  • CI パス確認
  • スキルレビューエージェントの指摘全件対応済み

Closes #25

- SKILL.md に Scope セクション追加(Core: Action 関連 / Reference: WF 全般)
- SKILL.md に Workflow-wide security tooling セクション追加
- security-checklist.md の各チェックに [Core] / [Reference] ラベルを付与
- Reference チェックに Auto-detection 情報を追記
- Automated Tooling セクション追加(ツール比較表)
- トリガーフレーズ調整("supply chain security audit" 追加)
- Output format を Title Case に統一

Closes #25
@claude
Copy link
Copy Markdown

claude Bot commented Mar 28, 2026

Claude encountered an error —— View job


I'll analyze this and get back to you.

@claude
Copy link
Copy Markdown

claude Bot commented Mar 28, 2026

Claude finished @yk-lab's task in 1m 29s —— View job


Code Review: PR #27

  • Gather context and read changed files
  • Review SKILL.md changes
  • Review security-checklist.md changes
  • Review upgrade-patterns.md changes
  • Post final review

Overall this is a well-structured improvement. The Core/Reference distinction adds meaningful clarity for both users and downstream skill-review agents. A few issues worth addressing before merge:


Issues

1. ghalint's action_ref_should_be_full_length_commit_sha listed under "Automated Tooling for Reference Checks"

security-checklist.md:186

| ghalint | Policy violations | `job_timeout_minutes_is_required`, `job_permissions`, `action_ref_should_be_full_length_commit_sha` |

action_ref_should_be_full_length_commit_sha is a Core check (SHA pinning = Check #1). Listing it in the "Reference Checks" tooling table implies SHA pinning is a reference-only concern that automated tooling handles — undermining the Core classification and potentially causing agents to deprioritize the manual audit step for SHA pinning.

Suggest either removing it from this table, or adding a note that this rule overlaps with Core check #1. Fix this →


2. pull_request_target severity inconsistency between Output Format and checklist

SKILL.md:35 vs security-checklist.md:50

The Output Format example categorizes pull_request_target as a Warning:

### Warning
- [ ] `release.yml:8` — Uses `pull_request_target` trigger

But the checklist places Check #3 (pull_request_target with Checkout) under Critical Issues. Agents following the Output Format example will emit this as a warning, contradicting the checklist's Critical classification. Fix this →


3. Scope section is buried after Notes in SKILL.md

SKILL.md:49

The Scope section appears at the very end, after Steps, Output Format, and Notes. An agent reading top-to-bottom executes all steps before encountering the scope boundary. This is the most important framing information — consider moving it before Steps so the agent knows what to audit (and what to defer to tooling) before starting.


Minor

4. ghalint rule name job_permissions may not be exact

security-checklist.md:186

The ghalint rule for permissions enforcement might be named differently (e.g., workflow_permissions_are_required or job_permissions_is_required). Worth verifying against ghalint's actual policy list before publishing, to avoid confusing users who look it up.


Approved with minor suggestions

The Core/Reference labeling, Auto-detection: annotations, and the tooling table are all good additions. Issues #1 and #2 have correctness implications and are worth fixing; #3 is a UX improvement worth considering.

- Scope セクションを Steps より前に移動(エージェントが先にスコープを把握できるように)
- pull_request_target を Output Format で Critical に移動(checklist と一致)
- ghalint テーブルから Core ルール (action_ref_should_be_full_length_commit_sha) を除外
- Notes の重複 Scope セクションを削除
@claude
Copy link
Copy Markdown

claude Bot commented Mar 28, 2026

Claude finished @yk-lab's task in 1m 17s —— View job


Code Review: PR #27

  • Read changed files
  • Review SKILL.md changes
  • Review security-checklist.md changes
  • Review upgrade-patterns.md changes
  • Post final review

Previous Review Issues — Status

All issues from the previous review have been addressed:

# Issue Status
1 action_ref_should_be_full_length_commit_sha in Reference tooling table ✅ Removed from ghalint row
2 pull_request_target in Warning block vs Critical classification ✅ Moved to Critical block in Output Format
3 Scope section buried after Notes ✅ Moved before Steps
4 ghalint rule name job_permissions unverified (minor) ⚠️ Still present, unverified

Review of Current State

SKILL.md

The restructured file flows well: Scope → Steps → Output Format → Notes → Tooling. The Output Format now correctly places pull_request_target under Critical, consistent with the checklist. The Scope section at the top is a meaningful improvement — agents know the audit boundary before executing.

One minor observation: the "Workflow-wide security tooling" block (### Workflow-wide security tooling) is nested under ## Notes by heading hierarchy, but semantically it's a separate concern. No action required — just noting it reads slightly awkwardly.

security-checklist.md

The Core/Reference labeling is clean and consistent. Check #2 (Script Injection) is correctly labeled [Reference] despite being in the Critical Issues section — the Scope explanation makes this distinction clear enough that it shouldn't confuse agents.

The tooling table is well-structured:

| ghalint | Policy violations | `job_timeout_minutes_is_required`, `job_permissions` |

Remaining minor issue (Issue #4 from previous review): job_permissions may not be the exact ghalint policy name. The ghalint docs list it as job_permissions in some versions but other names appear in different contexts. Worth a quick verification at ghalint's policy docs before this ships, to avoid user confusion when they look it up. Fix this →

upgrade-patterns.md

The added "Prerelease Versions" section is concise and useful. The default behavior (stable only, prerelease on explicit request) is a good conservative policy. No issues.


Verdict

Approved. All critical and structural issues from the previous review are resolved. The Core/Reference framing is coherent end-to-end across SKILL.md and security-checklist.md. The one remaining item (ghalint rule name verification) is cosmetic and low-risk — can be addressed in a follow-up if needed.

@claude
Copy link
Copy Markdown

claude Bot commented Mar 28, 2026

Claude encountered an error —— View job


I'll analyze this and get back to you.

skill-reviewer@claude-code-plugins が claude-code-plugins マーケットプレイスに存在せず
プラグインインストール失敗で CI が常に落ちていた。
prompt 内で plugin-dev:skill-reviewer エージェントを直接呼ぶ形で動作するため削除。
@claude
Copy link
Copy Markdown

claude Bot commented Mar 28, 2026

Claude finished @yk-lab's task in 1m 17s —— View job


Code Review: PR #27

  • Read changed files
  • Review SKILL.md changes
  • Review security-checklist.md changes
  • Review upgrade-patterns.md changes
  • Post final review

Previous Review Issues — Status

All issues from the previous two review rounds have been addressed:

# Issue Status
1 action_ref_should_be_full_length_commit_sha in Reference tooling table ✅ Removed
2 pull_request_target in Warning block vs Critical classification ✅ Moved to Critical in Output Format
3 Scope section buried after Notes ✅ Moved before Steps
4 ghalint rule name job_permissions unverified ⚠️ Still present

Review of Current State

.github/workflows/skill-quality-check.yml

skill-reviewer@claude-code-pluginsplugin-dev@claude-code-plugins — correct fix, consistent with the CLAUDE.md guidelines and commit e7fa405.

SKILL.md

The document structure is now: Scope → Steps → Output Format → Notes → Tooling. This is the right order — agents encounter the boundary conditions before executing.

One minor inconsistency: SKILL.md:64

- **ghalint** — Security policy enforcement (SHA pinning, permissions, timeouts)

The parenthetical mentions "SHA pinning," but SHA pinning is explicitly a Core check (not Reference/tooling). The Automated Tooling table in security-checklist.md correctly omits the SHA-pinning rule (action_ref_should_be_full_length_commit_sha) from ghalint. The SKILL.md description is stale relative to that fix and could mislead a user into thinking SHA pinning is something ghalint handles automatically (as a Reference check). Suggest changing to "permissions, timeouts" or "policy enforcement". Fix this →

security-checklist.md

Core/Reference labeling is clean and consistent across all 11 checks. The Auto-detection: lines on Reference checks (#2, #10, #11) add useful, actionable information. Tooling table structure is correct after removal of action_ref_should_be_full_length_commit_sha.

Remaining issue (Issue #4, third mention): security-checklist.md:186

| ghalint | Policy violations | `job_timeout_minutes_is_required`, `job_permissions` |

job_permissions as a ghalint policy name is unverified. The actual rule in ghalint may be named differently (e.g., workflow_permissions). Worth verifying against ghalint's policy list before this ships — users who look it up and find no match will lose trust in the tooling table. Fix this →

upgrade-patterns.md

The Prerelease Versions section is well-scoped: stable by default, prerelease only on explicit request. The blank-line formatting additions throughout improve readability. No issues.


Verdict

Approved. All critical and structural issues from prior reviews are resolved. Two minor items remain:

  1. The ghalint "SHA pinning" mention in SKILL.md (new, minor — consistency issue)
  2. The job_permissions rule name in the tooling table (carried over, unverified — low risk but worth a quick lookup)

Both are cosmetic and can be addressed as follow-ups if preferred.

@claude
Copy link
Copy Markdown

claude Bot commented Mar 28, 2026

Claude finished @yk-lab's task in 1m 43s —— View job


Code Review: PR #27

  • Read changed files
  • Review SKILL.md changes
  • Review security-checklist.md changes
  • Review upgrade-patterns.md changes
  • Review skill-quality-check.yml changes
  • Post final review

Previous Review Issues — Status

All issues from prior review rounds are resolved:

# Issue Status
1 action_ref_should_be_full_length_commit_sha in Reference tooling table ✅ Removed from ghalint row
2 pull_request_target in Warning block vs Critical classification ✅ Moved to Critical in Output Format
3 Scope section buried after Notes ✅ Moved before Steps
4 ghalint description mentioned "SHA pinning" (new issue from round 3) ✅ Fixed — now reads "permissions, timeouts"
5 job_permissions ghalint rule name unverified (minor, carried from round 1) ⚠️ Still unverified

Review of Current State

.github/workflows/skill-quality-check.yml

Plugin name corrected: skill-reviewer@claude-code-pluginsplugin-dev@claude-code-plugins. Clean fix.

skills/audit-actions/SKILL.md

Document structure is now: Scope → Steps → Output Format → Notes → Tooling — the right order for agent execution. Key points:

  • Scope section clearly separates Core (action-specific) from Reference (general workflow) concerns
  • Output Format correctly places pull_request_target under Critical, consistent with the checklist
  • ghalint description at line 64 correctly reads "permissions, timeouts" (SHA pinning removed)
  • The ### Workflow-wide security tooling block nested under ## Notes reads slightly awkward by heading hierarchy but is functionally fine

skills/audit-actions/references/security-checklist.md

Core/Reference labeling is clean and consistent across all 11 checks. Auto-detection: annotations on checks #2, #10, and #11 are accurate and actionable. The Automated Tooling table is well-structured after removal of action_ref_should_be_full_length_commit_sha.

One remaining minor item (Issue #5, fourth mention):

security-checklist.md:186

| ghalint | Policy violations | `job_timeout_minutes_is_required`, `job_permissions` |

The ghalint rule job_permissions has been flagged as unverified across all previous review rounds. I was unable to confirm the exact rule name against ghalint's policy docs in this review. The risk is low — the name is plausible and consistent with ghalint's naming convention — but worth a quick lookup before this ships to avoid user confusion. Fix this →

skills/upgrade-actions/references/upgrade-patterns.md

Blank-line spacing improvements throughout and a clean Prerelease Versions section added. The default behavior (stable unless explicitly requested) is the right conservative policy. No issues.


Verdict

Approved. All critical and structural issues from prior reviews are fully resolved. The Core/Reference framing is coherent end-to-end across SKILL.md and security-checklist.md, and the document structure guides agents correctly. The one remaining item (job_permissions rule name verification) is cosmetic and can be addressed as a follow-up if preferred.

@yk-lab yk-lab merged commit e5c35b2 into main Mar 28, 2026
6 of 7 checks passed
@yk-lab yk-lab deleted the fix/audit-actions-scope branch March 28, 2026 11:10
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.

feat: audit-actions のスコープ明文化 & zizmor/ghalint 導入推奨ハンドオフを追加

1 participant