feat: add skill validation CI job by CybotTM · Pull Request #31 · netresearch/github-project-skill

CybotTM · 2026-02-25T14:10:49Z

Summary

Add skill validation job calling reusable workflow from skill-repo-skill
Checks SKILL.md frontmatter, word count, composer.json, plugin.json, file presence

Test plan

CI passes with new validation job

Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>

gemini-code-assist · 2026-02-25T14:10:52Z

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

Copilot

Pull request overview

This PR adds a skill validation CI job that validates SKILL.md frontmatter, word count, composer.json, plugin.json, and file presence using a reusable workflow from the skill-repo-skill repository. The validation runs alongside existing lint jobs to ensure skill repository standards are met.

Changes:

Added a new validate job to the lint workflow that calls a reusable workflow from netresearch/skill-repo-skill

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@main

Address Copilot review finding from PR #31: pin the skill-repo-skill validate workflow to a specific commit SHA instead of @main for security and reproducibility.

…conversation resolution Add mechanical checkpoints (GH-30, GH-31) that verify enforce_admins and required_conversation_resolution via GitHub API, plus an LLM review checkpoint (GH-32) for the combined audit. Without enforce_admins, admins can bypass all branch protection rules including unresolved review threads. Updates verify script to check these settings and security-config.md reference to document the enforce_admins requirement.

…ecks GH-30/GH-31 checkpoint commands and the verify script now guard for gh CLI availability and valid REPO_SLUG before running. When classic branch protection is not configured, they fall back to querying GitHub rulesets for equivalent enforcement settings.

…limit

…allowlist The automated-assessment runner enforces a command allowlist that rejects command-chaining metacharacters (; && || backticks $()) and only accepts specific base commands. Four checkpoints failed allowlist validation and were never actually evaluated against target projects. GH-6 and GH-23 used `test -f X || test -f Y` chains to test for any of several files. Rewritten as `type: file_exists` with brace expansion, which is the runner's first-class idiom for "any of these files". GH-30 and GH-31 used multi-line YAML literal-block scalars (`target: |`) to invoke `gh api` for branch protection audits. The runner's simple line parser sees the literal-block indicator as the first token and rejects with "'|' not in allowed command whitelist". Even with the allowlist passing, these cannot be executed mechanically — they require GitHub API auth context. Converted to `type: gh_api`, which the runner recognises and skips with a clear evidence string. The semantically equivalent audit is preserved in GH-32 (llm_reviews).

…dition Addresses 3 of 10 review threads on PR #77 (Copilot threads on SKILL.md, checkpoints.yaml line 356, and the enforce_admins description inside repo-bootstrap.md). The init script requires the default branch ref to exist (exits 4 on empty repos). Earlier wording said 'before pushing the first commit' which is operationally impossible — the script needs a ref to protect. Corrected uniformly across: - SKILL.md 'When to Use' bullet: 'post gh repo create + initial push, before first PR' (was 'before first commit/PR') - references/repo-bootstrap.md intro paragraph: explicit three-step flow (create -> initial push -> protect, then open PRs) - checkpoints.yaml GH-31 desc: 'right after gh repo create + initial push, before opening the first PR' (was 'immediately after gh repo create') Also corrected repo-bootstrap.md's 'baseline deliberately omits' section: the template DOES include enforce_admins=false (explicit permissive default). Only required_signatures is omitted entirely. Section renamed to 'Deliberately permissive knobs' to match reality. Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>

Promote the new init-branch-protection.sh to a first-class surface in the skill: - 'When to Use' now leads with 'After gh repo create, before first commit/PR' (REQUIRED). - New 'Required First Step After gh repo create' section documents the two-step flow (apply baseline, then --from-current-checks after first CI run), links the snipe-it#17 incident as the pain that triggered this, and points at /assess github-project's GH-31 checkpoint for read-only verification. - 'Security & Compliance Quick Checks' gets a one-liner that prints whether required_conversation_resolution is enabled on the repo's default branch. - 'Running Scripts' lists init-branch-protection.sh alongside the existing verify-github-project.sh. The structural enforcement (required_conversation_resolution=true plus min-1-approver) is what makes the 'abort merge if unresolved threads' memory rule actually safe. Documentation without a runnable apply path demonstrably wasn't enough — operator discipline failed in snipe-it#17. Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>

…ode in GH-31 Severity was already 'error' (verified). This expands the desc to name the concrete user-visible failure mode that motivated promoting this to a hard checkpoint in the first place: - PR merges may silently include unresolved bot-reviewer feedback - Including security findings like token leakage in build logs - Concrete incident: netresearch/snipe-it-docker-compose-stack#17 The desc now also points operators at the new init script as the apply path, so /assess findings have an actionable remediation embedded in the failure message rather than a generic 'enable it'. Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>

…dition Addresses 3 of 10 review threads on PR #77 (Copilot threads on SKILL.md, checkpoints.yaml line 356, and the enforce_admins description inside repo-bootstrap.md). The init script requires the default branch ref to exist (exits 4 on empty repos). Earlier wording said 'before pushing the first commit' which is operationally impossible — the script needs a ref to protect. Corrected uniformly across: - SKILL.md 'When to Use' bullet: 'post gh repo create + initial push, before first PR' (was 'before first commit/PR') - references/repo-bootstrap.md intro paragraph: explicit three-step flow (create -> initial push -> protect, then open PRs) - checkpoints.yaml GH-31 desc: 'right after gh repo create + initial push, before opening the first PR' (was 'immediately after gh repo create') Also corrected repo-bootstrap.md's 'baseline deliberately omits' section: the template DOES include enforce_admins=false (explicit permissive default). Only required_signatures is omitted entirely. Section renamed to 'Deliberately permissive knobs' to match reality. Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>

…77) ## Summary Close the structural enforcement gap that just allowed three unresolved bot-reviewer threads — including a HIGH-severity token leak — to ship to `main` in [netresearch/snipe-it-docker-compose-stack#17](netresearch/snipe-it-docker-compose-stack#17). The skill already documented branch protection and already had a checkpoint (`GH-31`) for `required_conversation_resolution: true`, but there was nothing prompting an operator to actually apply protection at `gh repo create` time, before the first PR opens. By the time `/assess github-project` would have caught the miss, PRs were already merging. This PR adds a first-class init surface so the structural rule is applied at repo-creation time, not audited after the fact. ## What's in the PR - **`skills/github-project/scripts/init-branch-protection.sh`** — runnable, idempotent. Takes `<owner>/<repo>`, reads the new template, PUTs to `repos/{owner}/{repo}/branches/{default_branch}/protection`. Uses `gh api -X PUT --input -` with JSON body (NOT `--raw-field`, which silently breaks on nested JSON — that's exactly what wasted three retries in [snipe-it#15](netresearch/snipe-it-docker-compose-stack#15)). Handles empty-repo (exit 4), no access (exit 3), already-compliant (exit 0), and drift on opinionated fields (exit 1 with a per-field diff, no silent clobber). - **`skills/github-project/assets/branch-protection.json.template`** — synthesized from snipe-it's live post-fix state with two deliberate adjustments: `enforce_admins: false` explicit (so solo-maintainer repos like ldap-selfservice-… or usercentrics-widgets aren't painted into a corner), and `required_signatures` *omitted* (not set to `false`) so the script never resets a repo that has already opted into signing. Both are documented in the script header as per-repo tightening one-liners. - **`--from-current-checks` follow-up flag** — after the first CI run completes on the default branch, re-invoke with this flag and the script discovers the successful check-run names from the most recent completed run and PATCHes them in as required status contexts with `strict: true`. - **SKILL.md** — new \"Required First Step After \`gh repo create\`\" section right after \"When to Use\". The \"When to Use\" list now leads with the init step. \"Quick Diagnostics\" gets a one-liner (`gh api repos/OWNER/REPO/branches/.../protection --jq '.required_conversation_resolution.enabled'`). \"Running Scripts\" lists the new script alongside the existing verifier. - **`checkpoints.yaml` GH-31** — severity was already `error` (verified). The `desc:` now names the user-visible failure mode (silent unresolved-thread merges including token leaks like snipe-it#17) and points operators at the new script as the apply path. - **README.md** — 7-line callout at the top so someone landing on the repo page sees the init step without scrolling. ## The new flow ```bash gh repo create netresearch/new-thing --public --clone cd new-thing git push -u origin main # need at least one commit so the default branch ref exists bash ../github-project-skill/main/skills/github-project/scripts/init-branch-protection.sh \\ netresearch/new-thing # ... open PRs, run CI ... bash .../init-branch-protection.sh netresearch/new-thing --from-current-checks ``` ## What this does NOT fix (deliberately, follow-up) GitHub branch protection still cannot block on a *requested-but-pending* review. The snipe-it#17 leak slipped through the **unresolved-threads** class, which this PR's baseline (`required_conversation_resolution: true`) DOES close structurally. The **pending-reviewer** class (Copilot is mid-review, merge anyway) remains a workflow-discipline gap — see `references/security-config.md` lines 144-152. Closing it requires a status-check workflow (out of scope here). ## Verification before merge Run the unresolved-threads GraphQL check before any merge attempt: ```bash gh api graphql -f query='query{repository(owner:\"netresearch\",name:\"github-project-skill\"){ pullRequest(number:NUMBER){reviewThreads(first:100){nodes{isResolved}}} }}' --jq '.data.repository.pullRequest.reviewThreads.nodes | map(select(.isResolved == false)) | length' ``` Abort merge if non-zero. (This PR's own correctness is also a useful test of the rule it documents.) ## Test plan - [x] `shellcheck` clean on `init-branch-protection.sh` - [x] `bash -n` clean - [x] Live-tested against `netresearch/snipe-it-docker-compose-stack` — reports `already compliant` (idempotent path works against the repo whose incident motivated this PR) - [x] Error paths exercised: no arg → exit 2, bad slug → exit 2, 404 repo → exit 3, bad flag → exit 2 - [x] `markdownlint-cli2` clean on `SKILL.md` and `README.md` - [x] `yamllint` clean on `checkpoints.yaml` - [x] All four commits signed (verified via `git log --show-signature`) - [ ] CI green on this branch - [ ] Unresolved-threads GraphQL check returns 0 before merge ## Atomic commits 1. `feat(scripts): add init-branch-protection.sh for new-repo bootstrap` 2. `docs(skill): require branch-protection init step before first PR/commit` 3. `feat(checkpoints): clarify required_conversation_resolution failure mode in GH-31` 4. `docs(readme): announce required init step at top of README`

…emplate (#79) ## Why [PR #77](#77) shipped `assets/branch-protection.json.template` with two deliberate choices: - `enforce_admins: false` — solo-maintainer Netresearch repos benefit from admin-bypass in emergencies - `required_signatures` omitted — Dependabot/Renovate bot PRs without per-repo signing setup would otherwise be blocked `references/security-config.md` was already in the repo with stricter language: | Line | Old | Conflict | |---|---|---| | ~98 | `enforce_admins` **MUST be `true`** | Template ships `false` | | ~166 (table) | `required_signatures | true | Enforces GPG/SSH signed commits` | Template omits the field | Anyone reading the skill now would see the contradiction as either a documentation bug or as license to "fix" their per-repo config (and admin-bypass themselves out of an emergency-merge path). ## What changed - `enforce_admins` section: switched from "MUST be `true`" to "SHOULD be `true` on mature multi-maintainer repos as a hardening target". Added explicit acknowledgement that the init script ships `false` as the pragmatic baseline, plus the upgrade command + emergency-bypass rationale. - `required_signatures` table cell: now shows both states (target: `true`; init: unset) with the bot-signing precondition and per-repo upgrade trigger. - The under-`enforce_admins` security-note callout now points at the unresolved-threads operator-side safety valve for repos where the admin-bypass IS the right choice. ## What did NOT change - No template change. PR #77's `branch-protection.json.template` stays as-is. - No script change. The init flow + GH-31 checkpoint behaviour is unchanged. - No SKILL.md change. (Already at the 499-word ceiling; left it alone.) ## Test plan - [x] Markdown lint will run via CI on push - [x] Word count check: `wc -w skills/github-project/SKILL.md` = 499 (no change; this PR only touches `security-config.md`) - [ ] CI green ## Pre-merge gate I'll run the unresolved-threads GraphQL check before merging — the hard rule I just had to bake into memory after burning 3 PRs on the same mistake.

feat: add skill validation CI job

e66726b

Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>

CybotTM requested a review from Copilot February 25, 2026 14:11

Copilot started reviewing on behalf of CybotTM February 25, 2026 14:13 View session

CybotTM merged commit 6bf912e into main Feb 25, 2026
4 checks passed

CybotTM deleted the feat/add-skill-validation-ci branch February 25, 2026 14:14

Copilot AI reviewed Feb 25, 2026

View reviewed changes

Comment thread .github/workflows/lint.yml

CybotTM added a commit that referenced this pull request Mar 28, 2026

fix: pin reusable workflow reference to commit SHA

f6f725c

Address Copilot review finding from PR #31: pin the skill-repo-skill validate workflow to a specific commit SHA instead of @main for security and reproducibility.

CybotTM mentioned this pull request Mar 28, 2026

fix: pin reusable workflow reference to commit SHA #43

Merged

1 task

CybotTM mentioned this pull request Mar 30, 2026

feat: branch protection audit checkpoints (enforce_admins + conversation resolution) #49

Merged

4 tasks

CybotTM added a commit that referenced this pull request Mar 30, 2026

fix: break long jq filter line in GH-31 to satisfy yamllint 160-char …

703e3c6

…limit

CybotTM mentioned this pull request May 4, 2026

fix: rewrite checkpoint commands for runner allowlist; broaden license and metadata checks for org-wide files #69

Merged

3 tasks

CybotTM mentioned this pull request May 23, 2026

feat: require branch protection at repo init (close enforcement gap) #77

Merged

9 tasks

CybotTM mentioned this pull request May 23, 2026

docs(security-config): soften aspirational MUSTs to align with init template #79

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat: add skill validation CI job#31

feat: add skill validation CI job#31
CybotTM merged 1 commit into
mainfrom
feat/add-skill-validation-ci

CybotTM commented Feb 25, 2026

Uh oh!

gemini-code-assist Bot commented Feb 25, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

CybotTM commented Feb 25, 2026

Summary

Test plan

Uh oh!

gemini-code-assist Bot commented Feb 25, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants