Skip to content

feat(skills): add security scanning to cicd - #23

Merged
harrymove-ctrl merged 2 commits into
mainfrom
feat/cicd-security-validation
Aug 30, 2026
Merged

harrymove-ctrl merged 2 commits into
mainfrom
feat/cicd-security-validation

Conversation

@Abdol164

@Abdol164 Abdol164 commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

What's missing today

cmk:cicd frames itself as "three concerns that stay separated: what validates every change, what
ships a specific commit somewhere, and what gates or authenticates either one."
The pipeline it
composes validates build, test and docs — and never scans. No SAST, no dependency advisories, no
secret detection, no IaC scanning. "Security" appears in the skill only as where secrets are stored
and OIDC federation. Supply-chain coverage exists only inside cmk:rust; secrets are addressed as
storage, never as detection.

So a design can declare a control — cmk:design already requires a security section with
assumptions, gaps, and controls — and the pipeline has nowhere to enforce it.

The idea

Adds references/security-scanning.md: four classes (code, dependencies, secrets, infrastructure),
each a question rather than a product, satisfied by a scanner of that category or equivalent in
the cargo-deny style cmk:rust already uses.

Classes are scoped to the effort rather than run wholesale — running everything on every change
trains people to ignore the result. Selection reuses cmk:delivery-review's existing triad, applied
to classes rather than lenses:

  • Adaptive when unstated — select from what the diff touches; a docs-only diff implicates none.
  • Binding when stated — an explicit list honored exactly, in both directions.
  • Disclosed unconditionally — state which classes ran and which did not, each absence with its
    reason.

Deliberately not coupled to Quick/Targeted/Full. Depth measures how expensive a wrong answer is;
class selection follows which artifacts changed. Coupling them is wrong in both directions — a
Quick review of a lockfile bump still needs a full dependency scan, and a Full review of a
docs-only diff needs no SAST.

Secrets is the one class that always runs: a committed credential is not proportional to the size of
the change that carried it.

The load-bearing distinction

A findings list renders these identically, and they must not be reported the same way:

  • Not applicable — the diff touched no manifest, so the dependency class did not run. Coverage
    intact.
  • Could not run — the scanner was missing, unauthenticated, timed out, or loaded zero rules.
    Coverage has a hole.

Collapsing the second into the first is how a gate stops gating with nobody noticing. This is the
scanner-level form of a trap this skill already names: under skipped-job-reports-success the job
never ran; here it ran and checked nothing. Added to the traps list as
empty-scan-reads-as-clean, and separated by exit code — 3 for incomplete coverage, distinct
from 0.

Two smaller pieces

ci-structure.md gains the one place security departs from the surrounding convention: area
gating assumes blast radius matches file paths, which breaks here. A lockfile bump touches one file
and can alter the whole dependency graph; a credential lands in fixtures a src/** filter never
sees. Gate dependencies on manifests and lockfiles; run secrets unconditionally.

delivery-review lens 6 is "think like a bad actor with the diff in hand" — pure reasoning —
in a skill whose bar two sections later is "commands executed with output" and "a review with
zero findings and a thin evidence section is a failed review."
The lens now names the scans that
backed it and the classes that did not run, so it can meet the standard that file already sets.

That file sits at exactly 150 lines, so the two added lines are paid for by reflowing the
Boundary review paragraph, which left lens. alone on a line. No words changed — the diff
there is pure rewrap.

Suppressions

cmk:design requires stating security gaps. A suppression is that same accepted gap expressed where
it is enforced, so it carries the same obligation: a reason and an expiry, with an expired one
failing the gate. Accepted must not decay into forgotten.

Checks

  • bash scripts/skill-lint.sh — OK
  • Versions bumped: cmk:cicd 0.3.3 → 0.4.0 (new guidance), cmk:delivery-review 0.1.3 → 0.1.4;
    conventions.md roster updated
  • Companion docs updated for both skills
  • No tool mandated anywhere — every scanner reference reads "or equivalent"

Abdol164 and others added 2 commits August 28, 2026 22:20
The pipeline this skill composes validates build, test and docs and never
scans. Adds the four classes, scoped to what the diff touches and disclosed
either way, so a scan that could not run is not read as one that found
nothing — the scanner-level form of skipped-job-reports-success.

delivery-review's security lens now names the scans that backed it, so it
can meet the evidence bar that skill already sets.
Resolves the overlap with #22, which landed first:

- cicd/SKILL.md — keep main's "Use when…" description house form, carry this
  branch's "add security scanning" / "scan for vulnerabilities" triggers, and
  take 0.4.0 (a new guidance facet, not the 0.3.4 wording bump).
- delivery-review/SKILL.md — main moved the seven lenses out to
  references/lenses.md, so this branch's inline lens-6 edit had no target
  left. Re-applied it there instead: the security lens still has to name the
  scans that backed it and the classes that did not run. Taking either side
  of that conflict verbatim would have dropped the change silently.
- conventions.md — main's rewritten version roster, with cmk:cicd moved from
  the 0.3.x band to 0.4.x so it stays named exactly once.
- docs/ai/skills/delivery-review.md — record references/lenses.md and the new
  Red Flags / Rationalizations sections, which the lens extraction left out.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@harrymove-ctrl
harrymove-ctrl merged commit 645c596 into main Aug 30, 2026
2 checks passed
harrymove-ctrl added a commit that referenced this pull request Aug 30, 2026
…-validation"

This reverts commit 645c596, reversing
changes made to 6b6781f.
@harrymove-ctrl

Copy link
Copy Markdown
Contributor

Reverted from main in ae4ae31, together with #22 which it was merged on top of. Continues in #26 with @Abdol164's commit unchanged (identical diff: 7 files, +177/-23). Please review there.

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.

3 participants