diff --git a/docs/ai/skills/cicd.md b/docs/ai/skills/cicd.md index dbaf470..b95faca 100644 --- a/docs/ai/skills/cicd.md +++ b/docs/ai/skills/cicd.md @@ -9,7 +9,11 @@ policy gates plus automation credentials that govern both. ## Approach CI structure: a `changes` job path-filters the diff and gates per-area jobs, with concurrency-per-ref, ecosystem caching, config-selected runner tiers, and -a scheduled, deliberately cache-isolated cold-resolve job. Deploy and release: +a scheduled, deliberately cache-isolated cold-resolve job. Security scanning +sits inside that pipeline as four classes — code, dependencies, secrets, +infrastructure — scoped to what the diff touches but disclosed either way, +with the secret scan exempt from path gating and a scan that could not run +failing its gate rather than reading as clean. Deploy and release: `workflow_dispatch` inputs carry an explicit commit SHA and target environment (dispatch-against-ref), a deploy orchestrator calls per-piece `workflow_call` legs under two concurrency layers, and irreversible releases @@ -22,7 +26,8 @@ where pushes must trigger CI, and OIDC over static cloud keys. Specifies the GitHub ↔ IaC 1:1:1 mapping (stack ↔ GitHub Environment ↔ deploy workflow) that `cmk:infra` names but leaves to this skill to wire. Names speedup misattribution as a named trap alongside cold-cache poisoning, -skipped-job-reports-success, workflow-token-doesn't-trigger-CI, +skipped-job-reports-success, empty-scan-reads-as-clean, +workflow-token-doesn't-trigger-CI, evidence-floor negation, and composer-contract drift. Host-runnable scripts prefer TypeScript (or the repo's existing runtime); shell is host bootstrap only; a shared mutating CLI refuses without `--confirm` @@ -39,7 +44,12 @@ repo. (TypeScript default, shell bootstrap only, no third ops language), composer-contract drift, mutate gate, host matrix, JIT slot count, one-fix-per-push. -- `references/ci-structure.md` — the `changes` job/path-filter shape, +- `references/security-scanning.md` — the four scan classes, scoping them to + the effort, not-applicable vs. could-not-run, per-class placement, exit + codes, suppressions as accepted gaps with an expiry, proving the gate + gates. +- `references/ci-structure.md` — the `changes` job/path-filter shape, the + security job as the exception to path gating, concurrency and caching, config-selected runner tiers, deliberate cold-cache isolation, CI self-contract tests (implementation site and path-filter lockstep), label-gated diagnostics. diff --git a/docs/ai/skills/conventions.md b/docs/ai/skills/conventions.md index c3e6ac1..34bb4b1 100644 --- a/docs/ai/skills/conventions.md +++ b/docs/ai/skills/conventions.md @@ -8,7 +8,7 @@ Frontmatter declares three fields the host (Claude Code or OpenCode) reads to di - `name` — `cmk:`, used as the slash command and skill ID. - `description` — opens in the second person (`Use when…` / `Use whenever…`) with trigger phrases plus an **outcome noun** (the deliverable), not a workflow step list. Used by the agent to auto-select the skill from user intent. A user-invoked skill (`disable-model-invocation: true`) writes one plain human-facing line naming the deliverable instead — the agent never routes on that line. -- `version` — every skill appears in exactly one band below; a bump moves it between bands rather than adding a second mention. `0.6.x`: `cmk:design`. `0.5.x`: `cmk:delivery-pipeline`. `0.4.x`: `cmk:local-stack`, `cmk:requirements` (Standard elicitation: close package, scope band, guards), `cmk:rule`. `0.3.x`: `cmk:adr`, `cmk:agent-instructions`, `cmk:cicd`, `cmk:delivery-ship`, `cmk:delivery-spec-plan`, `cmk:delivery-workflow`, `cmk:docs`, `cmk:infra`. `0.2.x`: `cmk:agent-vendors`, `cmk:delivery-handoff`, `cmk:delivery-intake`, `cmk:delivery-review`, `cmk:enclave`, `cmk:learn`, `cmk:mcp-config`, `cmk:project-layout`, `cmk:toolchain`. `0.1.x`: `cmk:codebase-docs`, `cmk:delivery-simplify`, `cmk:discover-efforts`, `cmk:glossary`, `cmk:interpret`, `cmk:repo-setup`, `cmk:rust`, `cmk:sui-devstack`, `cmk:sui-sdk`, `cmk:sync`, `cmk:test-resources`, `cmk:testcontainers`. +- `version` — every skill appears in exactly one band below; a bump moves it between bands rather than adding a second mention. `0.6.x`: `cmk:design`. `0.5.x`: `cmk:delivery-pipeline`. `0.4.x`: `cmk:cicd` (security-scanning facet), `cmk:local-stack`, `cmk:requirements` (Standard elicitation: close package, scope band, guards), `cmk:rule`. `0.3.x`: `cmk:adr`, `cmk:agent-instructions`, `cmk:delivery-ship`, `cmk:delivery-spec-plan`, `cmk:delivery-workflow`, `cmk:docs`, `cmk:infra`. `0.2.x`: `cmk:agent-vendors`, `cmk:delivery-handoff`, `cmk:delivery-intake`, `cmk:delivery-review`, `cmk:enclave`, `cmk:learn`, `cmk:mcp-config`, `cmk:project-layout`, `cmk:toolchain`. `0.1.x`: `cmk:codebase-docs`, `cmk:delivery-simplify`, `cmk:discover-efforts`, `cmk:glossary`, `cmk:interpret`, `cmk:repo-setup`, `cmk:rust`, `cmk:sui-devstack`, `cmk:sui-sdk`, `cmk:sync`, `cmk:test-resources`, `cmk:testcontainers`. - `disable-model-invocation: true` — optional, fourth field only. Present on `cmk:interpret`. The closer is still `---`. No skill file references outside its own package by relative path — the rule binds a package's own references, not content it emits into a target repo; a skill that needs a target-repo artifact names it repo-root-relative, and a skill that needs another skill cites it by `cmk:` name — see `cmk:agent-vendors`. diff --git a/docs/ai/skills/delivery-review.md b/docs/ai/skills/delivery-review.md index 714ca4d..e7b9760 100644 --- a/docs/ai/skills/delivery-review.md +++ b/docs/ai/skills/delivery-review.md @@ -15,14 +15,18 @@ lenses are correctness, spec/design/requirements/AC compliance, code quality, cross-surface consistency, edge cases, security, and production readiness. Every lens writes `file:line` findings plus a "what I read / ran / checked" evidence trail — a review with no findings and no evidence trail -is a failed review. Findings are adversarially verified (the -`cmk-delivery-verifier` role) before disposition: fix now, rescope the -criterion, defer with a tracker issue, or discard with a reason. +is a failed review; the security lens additionally names the scans that +backed it and the classes that did not run (`cmk:cicd`). Findings are +adversarially verified (the `cmk-delivery-verifier` role) before +disposition: fix now, rescope the criterion, defer with a tracker issue, or +discard with a reason. ## Where - Skill body: `skills/delivery-review/SKILL.md` — sections Review depth, The two occasions, The lenses, Evidence or it did not happen, Verify - before acting, Disposition. + before acting, Disposition, Red Flags, Rationalizations. +- `references/lenses.md` — the seven lenses in full, including the scans the + security lens must name and the classes it must report as not run. - `references/engines.md` — choosing or running a review engine at full depth. - `references/linear.md` — the skill body's conditional tracker-binding diff --git a/skills/cicd/SKILL.md b/skills/cicd/SKILL.md index fd8cb59..e7307b5 100644 --- a/skills/cicd/SKILL.md +++ b/skills/cicd/SKILL.md @@ -1,7 +1,7 @@ --- name: cmk:cicd -description: Use when the user asks to "set up CI", "speed up CI", "add a deploy workflow", "structure GitHub Actions", "self-hosted runners", "run CI locally", "JIT runner", "protect the main branch", or needs to structure CI, deployment, and policy automation as composable host-runnable scripts that GitHub Actions only automates. -version: 0.3.4 +description: Use when the user asks to "set up CI", "speed up CI", "add a deploy workflow", "structure GitHub Actions", "self-hosted runners", "run CI locally", "JIT runner", "protect the main branch", "add security scanning", "scan for vulnerabilities", or needs to structure CI, deployment, and policy automation as composable host-runnable scripts that GitHub Actions only automates. +version: 0.4.0 --- # CI/CD @@ -12,8 +12,9 @@ somewhere, and what gates or authenticates either one. ## Modes -**Init** (default) — stand up the path-filtered CI pipeline, per-environment -deploy workflows, policy gates, and the workflows README. +**Init** (default) — stand up the path-filtered CI pipeline, its security +scans, per-environment deploy workflows, policy gates, and the workflows +README. **Update** — add, rename, or retire a workflow; update the README table and required checks in the same change. @@ -24,8 +25,9 @@ never mutates. ## Three facets, one split - **CI structure** — one path-filtered validation pipeline gating everything - that runs on a push or PR. Read `references/ci-structure.md` when setting up - or speeding up CI. + that runs on a push or PR, security scans included. Read + `references/ci-structure.md` when setting up or speeding up CI, and + `references/security-scanning.md` when adding or gating a scan. - **Deploy & release** — dispatch-against-ref deployment of a reviewed commit to a named environment, plus release integrity for anything irreversible. Read `references/deploy-and-release.md` when adding or changing a deploy or @@ -81,9 +83,13 @@ scheduled cold job whose own setup step silently re-warms a shared cache, so the regression it exists to catch can no longer show up); **skipped-job- reports-success** (a path-filtered job that didn't run still reports a green check, so the gating job itself, not the per-area jobs, must be what's -required); **workflow-token-doesn't-trigger-CI** (automation that pushes with -the built-in workflow token produces commits that never fire downstream CI, -silently leaving a rewritten ref unverified); **speedup misattribution** (a +required); **empty-scan-reads-as-clean** (the same failure one level in — a +scanner missing its binary, its credentials, or its ruleset returns zero +findings, which is indistinguishable from a passing scan, so coverage must +be reported separately from findings); **workflow-token-doesn't-trigger-CI** +(automation that pushes with the built-in workflow token produces commits +that never fire downstream CI, silently leaving a rewritten ref +unverified); **speedup misattribution** (a multi-part change to CI's wall-clock cuts the total, and every part gets credited — but tracing the win to its actual cause can reveal that one part did all of it and a sibling part is silently inert, contributing nothing @@ -114,6 +120,12 @@ Report-only — never mutate: actual workflow job names — no drift between the two. - No long-lived cloud credential sits in a secret where OIDC federation is available. +- A secret-scanning job runs on every pipeline invocation, not gated on + changed paths, and the dependency scan also runs on a schedule. +- A scan that could not run fails its gate; the exit code for incomplete + coverage is distinct from the one for a clean run. +- Every scan suppression carries a reason and an expiry, and an expired one + fails the gate. - `.github/workflows/README.md` lists every workflow and is current with the workflows on disk. - A claimed speedup names the specific job, step, or mechanism it traces to — diff --git a/skills/cicd/references/ci-structure.md b/skills/cicd/references/ci-structure.md index ca00a76..809b496 100644 --- a/skills/cicd/references/ci-structure.md +++ b/skills/cicd/references/ci-structure.md @@ -45,6 +45,18 @@ jobs: steps: [ ... ] ``` +## The security job is the exception to path gating + +Area gating assumes a change's blast radius matches its file paths. That +holds for build and test jobs and breaks for security: a lockfile bump +touches one file and can alter the whole dependency graph, and a credential +lands in fixtures a `src/**` filter never sees. + +Gate the dependency scan on manifests and lockfiles rather than on the area +that owns them, and run the secret scan unconditionally — no `if:` on a +`changes` output. Read `references/security-scanning.md` for the class list +and what each gate does with a scan that could not run. + ## Speed structure - **Concurrency group per ref**, `cancel-in-progress` on pull requests only — diff --git a/skills/cicd/references/security-scanning.md b/skills/cicd/references/security-scanning.md new file mode 100644 index 0000000..1f0c2b8 --- /dev/null +++ b/skills/cicd/references/security-scanning.md @@ -0,0 +1,118 @@ +# Security scanning + +Scanners fail quiet. An absent binary, an expired token, a ruleset that +never shipped — each returns an empty finding list indistinguishable from a +clean result, and the gate stays green while checking nothing. Everything +here follows from that. + +## Four classes + +A class is a question, not a product. Each is answered by a scanner of that +category or equivalent; name the chosen one in `docs/rules/`. + +| Class | Answers | Reads | +|---|---|---| +| Code | injection, authz gaps, unsafe APIs, secrets in logic | source | +| Dependencies | known advisories, malicious packages, license policy | manifests, lockfiles | +| Secrets | credentials committed to the tree or its history | the diff; history on adoption | +| Infrastructure | misconfigured IaC, over-permissioned roles, image findings | IaC, Dockerfiles, workflows | + +## Selecting classes + +Running every class on every change trains people to ignore the result. +Scope them the way `cmk:delivery-review` scopes lenses: + +- **Adaptive when unstated** — select from what the diff touches. Source + implicates code; a changed manifest or lockfile implicates dependencies; + IaC or workflow edits implicate infrastructure. A docs-only diff + implicates none. +- **Binding when stated** — an explicit class list is honored exactly, in + both directions. Where it contradicts a risk signal, run what was asked + and name the unacted signal. +- **Disclosed unconditionally** — state which classes ran and which did not, + each absence with its reason. No reader recovers that from a findings + list. + +Secrets is the exception and always runs: a committed credential is not +proportional to the size of the change that carried it. + +Escalate to every class when the effort changes a trust boundary, an +authn/authz path, a signed or serialized wire format, a deployment surface, +or the release pipeline itself. + +## Not applicable is not could not run + +Two absences a findings list renders identically: + +- **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. A run with +any *could not run* class is incomplete, never clean, and says so before it +lists a finding. This is `empty-scan-reads-as-clean`, the scanner-level form +of `skipped-job-reports-success`: there the job never ran, here it ran and +checked nothing. + +## Placement + +| Class | Runs | Why there | +|---|---|---| +| Secrets | pre-commit and CI | Cheapest scan, unrecoverable miss. Hooks are bypassable, so CI repeats it. | +| Code | pull request | Needs the diff. A whole-tree pass belongs on a schedule. | +| Dependencies | pull request on manifest/lockfile change, and on a schedule | Advisories are disclosed against code that has not changed. | +| Infrastructure | pull request on IaC, container, workflow change | Cheap to catch, expensive to deploy. | + +The scheduled dependency run is the one usually skipped and the only one +that catches an advisory published since the last merge. + +Path gating is the one pipeline convention these jobs do not follow; +`references/ci-structure.md` covers how they are wired instead. + +## Exit codes + +| Code | Meaning | CI treats as | +|---|---|---| +| 0 | Selected classes ran, nothing at or above threshold | pass | +| 1 | Findings at or above threshold | fail | +| 2 | Scan could not run at all | fail | +| 3 | Ran, but a class could not run | fail | + +If a gate must be non-blocking during adoption, make it non-blocking for +findings and keep it blocking for coverage — the opposite of the usual +choice, and the one that keeps the gate honest while a backlog is worked +down. + +## Suppressions are accepted gaps + +`cmk:design` requires a security section stating assumptions, gaps, and +controls. A suppression is that same accepted gap expressed where it is +enforced, so it carries the same obligation: a reason, and an expiry. An +expired suppression fails the gate rather than renewing silently. Accepted +must not decay into forgotten. + +On adoption, baseline the existing findings rather than blocking on them — +a fresh gate against an accumulated backlog gets disabled within a week — +then block on new findings from day one. + +## Prove the gate gates + +A security job's failure mode is silence, so it needs the same evidence any +critical path does: + +- Commit a canary — a known-vulnerable dependency or an obvious injection on + a scratch branch — and confirm the gate fails. +- Fail the job when the scanner reports an implausibly low rule count. +- Read the coverage line, not the conclusion. Every class reporting *could + not run* under a green check is a failing scan wearing a passing badge. +- On a codebase with history, a gate that has never once failed is evidence + of breakage rather than cleanliness. + +## Reporting + +Emit SARIF or equivalent so findings land in the review surface. Every +result must reference a rule the report also declares — consumers silently +drop results whose rule they cannot resolve, and the upload still succeeds. +Carry the coverage report into the format's notification channel so a +consumer can tell a clean scan from one that did not run. diff --git a/skills/delivery-review/SKILL.md b/skills/delivery-review/SKILL.md index babec38..803b024 100644 --- a/skills/delivery-review/SKILL.md +++ b/skills/delivery-review/SKILL.md @@ -53,11 +53,10 @@ Choosing or running a review engine at full depth? Read `references/engines.md`. ## The two occasions -**Boundary review** — a concrete cross-work or risk boundary: a join of -two or more issues or branches, a shared or public contract, a persisted -format, a deployable-component handoff, an upstream pin replacement, or a -changed consumer interface. State the trigger and inspect every relevant -lens. +**Boundary review** — a concrete cross-work or risk boundary: a join of two +or more issues or branches, a shared or public contract, a persisted format, +a deployable-component handoff, an upstream pin replacement, or a changed +consumer interface. State the trigger and inspect every relevant lens. **Pre-ship review** — mandatory before every completed issue or branch ships, and before every combined integration output; `cmk:delivery-ship` diff --git a/skills/delivery-review/references/lenses.md b/skills/delivery-review/references/lenses.md index 94b5632..5ce487a 100644 --- a/skills/delivery-review/references/lenses.md +++ b/skills/delivery-review/references/lenses.md @@ -31,7 +31,8 @@ 6. **Security** — think like a bad actor with the diff in hand: injection, authz gaps, trust-boundary confusion, resource exhaustion, secret handling, and anything touching signed payloads, domain separators, - settlement, randomness, or wire parity. + settlement, randomness, or wire parity. Name the scans that backed this + lens and the classes that did not run (`cmk:cicd`). 7. **Production readiness** — audit against `cmk:delivery-pipeline`'s engineering-principles checklist and the spec's production-readiness section: failure modes, config, secrets, migrations, observability,