git:20260809.786f3c8 to git:20261016.63d2b02

+ description: Infrastructure code review — supply-chain pinning, secret exposure, workflow triggers and untrusted input, container hygiene, deployment safety. Load when reviewing a diff that touches CI/CD workflows, Dockerfiles, deployment configs or IaC.

+ > **Quick Guide:** Review operational code for what it can leak and what it lets a stranger run. Secrets, mutable third-party references and outsider input reaching a privileged job block the merge in any diff, at any scale; deployment ceremony scales with what the diff actually deploys. Recent facts that change the answer: `actions/checkout` v7 refuses fork-PR code under `pull_request_target` and `workflow_run` unless told otherwise; since December 2025 `pull_request_target` always runs the default branch's copy of the workflow; an organisation can enforce SHA pinning so a tag-pinned action fails at job setup; and GitHub's workflow dependency lockfile (public preview) pins what a `uses:` tag resolves to.

+ **Detailed Resources:**

+

+ - [examples/core.md](examples/core.md) — a good/bad pair for each pattern below: pinned actions and installs, secret mounts and log hygiene, permissions, privileged triggers and script injection, the non-root Dockerfile, deploy ordering, signal-safe startup

+

+ ## Before reviewing infrastructure code

+ **Grep the diff for credentials before reading it for anything else.** A pushed secret stays reachable through clones, forks and the pull request's own refs, so the fix to ask for is revocation, not a follow-up commit — and the check takes seconds.

+ **Resolve every external reference to something immutable.** A tag or branch on an action, base image, provider or module runs whatever its owner — or whoever holds the owner's token — points it at next, with this pipeline's secrets in scope.

+ **Trace each secret from its store to the one process that needs it.** A build arg lands in image history, a job-level `env:` reaches every step including third-party actions, and log masking matches only the exact registered string — each hop the diff adds is a place it leaks.

+ **Read a workflow's trigger and `permissions:` before its steps.** Under `pull_request_target` and `workflow_run` the job holds the base repository's secrets and token while handling fork-controlled input, and a missing `permissions:` block inherits a default set in settings the diff never shows.

+ **Treat every value an outsider can set as code.** A PR title, branch name or comment body written as `${{ }}` inside `run:` is spliced into the script before the shell parses it, so a quote in that value ends the string and the rest runs as commands.

+ **Check that a production image ends on a non-root `USER`.** Without one the process runs as root, and an application compromise starts one container-escape bug away from the host.

+ **Auto-detection:** `.github/workflows`, `uses:`, `permissions:`, `pull_request_target`, `workflow_run`, `GITHUB_TOKEN`, `secrets.`, `allow-unsafe-pr-checkout`, `concurrency:`, `.gitlab-ci.yml`, `Dockerfile`, `.dockerignore`, `compose.yaml`, `FROM`, `USER`, `ARG`, `RUN --mount=type=secret`, Kubernetes manifest, Helm chart, `readinessProbe`, `terraform`, `.terraform.lock.hcl`, Pulumi, CI review, Dockerfile review, IaC review

+ **Applies to:**

+ - Diffs touching CI/CD workflow files — triggers, permissions, pinned references, secrets, job ordering

+ - Dockerfiles, `.dockerignore` and compose files

+ - Deployment manifests and platform configs — readiness, resources, shutdown, per-environment config

+ - IaC — provider and module pinning, state, plan and apply in CI

+ - Release and build scripts — what they publish, sign and tag, and which credentials they read

+ - Lockfile and package-manager changes with supply-chain reach

+ **Handled elsewhere:**

+ - Application code in the same diff — reviewed against its own layer's conventions; this skill reviews what builds, ships and runs it.

+ - Severity vocabulary, comment format and the approve-or-request-changes call — the general review practice, which this skill's severities slot into.

+ - Designing or restructuring a pipeline — a review names the defect and its fix; the platform's shape belongs to whoever owns it.

+ - Incident response and live operations — a review gates a diff, not a running system.

+ ---

+ <philosophy>

+ ## Philosophy

+ Operational code fails in production only. No unit test catches an unpinned action's compromise or a leaked deploy key, so the review is often the one gate this code passes — and a finding costs a line before merge and a rotation after.

+ Ceremony scales with blast radius: a production deploy path earns strictness, a lint workflow that deploys nothing does not. Read the diff against what it reaches —

+ - A new env var must exist in every environment the app runs in; a renamed job or output must change everywhere that `needs` or reads it.

+ - Ask what the pipeline does on a fork PR, on a re-run, and on two branches at once.

+ **Don't flag:**

+ - Probes, resource limits or rollback strategy on a docs site or a workflow that deploys nothing

+ - A multi-stage build for a CI-only image whose size nothing depends on

+ - Missing caching in a job that runs in seconds

+ - A vault migration in a diff that consumes an existing secret the established way

+ </philosophy>

+ ---

+ <decision_framework>

+ ## Severity for infrastructure findings

+ Must Fix blocks approval. Should Fix travels with the approval, for the author to fix before merging, and needs a cost concrete in this diff. Nice to Have is the author's to take or leave.

+ ```

+ Does the diff introduce a security defect?

+ ├─ Credential literal, or a secret through ARG/ENV, a log line or an artifact → MUST FIX

+ ├─ Third-party action on a tag, or any action on a branch (@v3, @main) → MUST FIX

+ ├─ ${{ }} of outsider-controlled input inside run: → MUST FIX

+ ├─ pull_request_target / workflow_run that checks out or runs PR head code → MUST FIX

+ ├─ Write scopes a job doesn't use, or no permissions: on a workflow fed untrusted input → MUST FIX

+ ├─ Production image without a non-root USER → MUST FIX

+ └─ NO → Is it an operational-correctness gap?

+ ├─ GitHub's own action (actions/*) on a version tag instead of a full SHA → SHOULD FIX

+ ├─ Base image on latest or untagged, or without a digest on a deploy path; CI install that ignores the lockfile → SHOULD FIX

+ ├─ No permissions: block on any other workflow → SHOULD FIX

+ ├─ Debug output beside secret use (set -x, env dumps) → SHOULD FIX

+ ├─ Deploy job without a concurrency group, or not waiting on the tests → SHOULD FIX

+ ├─ New env var missing from one environment → SHOULD FIX

+ ├─ Cache key that never changes; cache-hostile Dockerfile layer order → SHOULD FIX

+ ├─ New Dockerfile without .dockerignore; shell-form CMD or CMD via npm → SHOULD FIX

+ ├─ New production service without a readiness check, resource requests or limits → SHOULD FIX

+ └─ NO → Is it a genuine enhancement?

+ ├─ Slimmer base image where size demonstrably matters → NICE TO HAVE

+ ├─ Faster caching for an already-fast job → don't write it

+ ├─ Production ceremony on a workflow that deploys nothing → don't write it

+ └─ Tool preference (compose vs Kubernetes, one package manager over another) → don't write it

+ ```

+ </decision_framework>

+ ## Core patterns

+ Every external reference resolves to something immutable, and the record of it is committed.

+ - Third-party actions on a full 40-character commit SHA from the action's own repository, not a fork; a trailing `# vX.Y.Z` comment keeps it readable

+ - GitHub's own `actions/*` on a full SHA as well — GitHub's secure-use guidance makes no first-party exception, and an organisation's SHA-pinning policy fails any unpinned action at job setup. A version tag here is Should Fix rather than Must Fix, because GitHub is a smaller trust grant than a third party

+ - Base images on a version tag at minimum, and on a digest on deploy paths — `latest` changes the build with no diff to review, and a publisher can re-point any tag; a digest is the only immutable form

+ - CI installs from the committed lockfile and fails on drift (`npm ci`, or the package manager's frozen-lockfile mode)

+ - Terraform providers and modules carry version constraints, and `.terraform.lock.hcl` is committed

+ # Must Fix: whoever controls this repository's tags decides what runs next

+ # Good: immutable; the comment keeps it readable

+ Full code: [examples/core.md](examples/core.md#pattern-1-supply-chain-pinning)

+ - No literal tokens, keys, passwords or connection strings — grep for `token`, `key`, `secret`, `password` and `://user:pass@` shapes

+ - Secrets come from the platform's secret store, never a committed file

+ - No secret in a Dockerfile `ARG` or `ENV` — a secret mount exists for one `RUN` and in no layer

+ - No secret in workflow- or job-level `env:` when one step uses it — scope it to that step

+ - No line prints a secret or a value derived from one; derived values are registered with `::add-mask::`

+ - Artifacts and caches cannot contain `.env` files, credentials, Terraform state or saved plans

+ - `.gitignore` and `.dockerignore` cover the `.env` and credential paths the diff introduces

+ # Must Fix: the token is in `docker history` and in the .npmrc layer

+ # Good: the credentials file is mounted for this one RUN

+ RUN --mount=type=secret,id=npmrc,target=/root/.npmrc npm ci

+ Full code: [examples/core.md](examples/core.md#pattern-2-secret-exposure)

+ ### Pattern 3: Workflow Trust Boundaries

+ What a workflow can reach is set by its trigger and its `permissions:`; what an outsider can make it do is set by where their input lands.

+ - `permissions:` read-only (or `{}`) at workflow level, write scopes granted per job

+ - `pull_request_target` and `workflow_run` only where the job needs the base repository's secrets or write token, and never checking out or running PR head code

+ - Under those triggers: no `allow-unsafe-pr-checkout: true`, no `actions/checkout` older than v7, no manual fetch of the PR head — v7's refusal guards only its own default path

+ - Outsider-set values — `github` context fields ending in `title`, `body`, `head_ref`, `label`, `message`, `name`, `email` or `ref` — reach `run:` through `env:`, never as `${{ }}` in the script

+ ```yaml

+ # Must Fix: the title is pasted into the script before bash parses it

+ - run: echo "Checking ${{ github.event.pull_request.title }}"

+ # Good: the shell receives data, not script

+ - env:

+ PR_TITLE: ${{ github.event.pull_request.title }}

+ run: echo "Checking $PR_TITLE"

+ Full code: [examples/core.md](examples/core.md#pattern-3-workflow-trust-boundaries)

+

+ ### Pattern 4: Dockerfile Hygiene

+

+ The image is minimal, cache-friendly and unprivileged.

+

+ - Production stage ends on a non-root `USER`

+ - A multi-stage build keeps compilers and dev dependencies out of the image that ships

+ - Dependency manifests are copied and installed before the source, or every code change reinstalls everything

+ - `.dockerignore` excludes `.git`, `.env` and local dependency folders — `COPY . .` copies whatever the context holds

+ - Base image minimal for the job (slim, alpine, distroless) where the app's native dependencies allow

+

+ # Should Fix: every source change busts the dependency layer

+ # Good: the install layer is reused until a manifest changes

+ Full code: [examples/core.md](examples/core.md#pattern-4-dockerfile-hygiene)

+ ### Pattern 5: Pipeline Correctness

+ The jobs compose the way the author thinks they do.

+ - `needs:` matches real dependencies — deploy waits for every test job, not only lint

+ - Deploy jobs share a `concurrency` group, so two quick merges cannot deploy out of order

+ - Cache keys include a lockfile hash (`hashFiles(...)`); a static key never invalidates

+ - A renamed job, output or env var is renamed at every reference in the same diff

+ concurrency:

+ group: deploy-production

+ cancel-in-progress: false

+ jobs:

+ deploy:

+ needs: [lint, test, e2e]

+ Full code: [examples/core.md](examples/core.md#pattern-5-pipeline-correctness)

+ ### Pattern 6: Deployment Safety, Scoped to the Diff

+ When the diff changes how production runs, check it can fail safely.

+ - Services behind a load balancer or orchestrator have a readiness check — without one, traffic reaches a container still booting

+ - New containers on a shared cluster declare resource requests and limits — requests are what the scheduler places by, and a memory limit makes a leak kill its own container rather than starve its neighbours

+ - The app receives SIGTERM and finishes in-flight work inside the grace period: exec-form `CMD`, not shell form or `npm start`, and a handler or an init process, because PID 1 gets no default signal behaviour

+ - Every env var and secret the diff introduces exists in every environment the app deploys to

+ - IaC backend and locking changes are deliberate, a risky change (a backend move, a destroy or a replace) comes with its `terraform plan` output so the reviewer sees what the apply will do, and `apply` in CI runs the plan CI produced against the real state

+ ```dockerfile

+ # Should Fix: /bin/sh -c is PID 1 and does not pass SIGTERM on

+ CMD node dist/server.js

+ # Good: the app receives the signal itself

+ CMD ["node", "dist/server.js"]

+ Full code: [examples/core.md](examples/core.md#pattern-6-deployment-safety-scoped-to-the-diff)

+ ## Red flags

+ **Common mistakes:**

+ - SHA-pinning `actions/*` while a third-party action stays on a tag — backwards, since the third party is the larger trust grant

+ - Asking only for a leaked secret's removal from the diff — it is already pushed, so ask for revocation

+ - "Fixing" a slow image build by dropping the lockfile instead of reordering the layers

+ - Approving an `apply` in CI because a plan passed locally — against different state it proves nothing about this run

+ **Surprising behaviour:**

+ - Build-arg values appear in `docker history` and in max-mode provenance attestations.

+ - Log masking redacts the exact registered value; a base64, URL-encoded or substring form prints in clear, and a secret stored as a JSON blob is not reliably masked.

+ - Every step in a job sees workflow- and job-level `env:`, third-party actions included.

+ - A job's `permissions:` block is that job's whole grant: every scope it omits is `none`, whatever the workflow level said.

+ - `GITHUB_TOKEN`'s default comes from organisation and repository settings the diff doesn't show; fork PRs get read-only unless the repository sends write tokens to forks.

+ - Since 8 December 2025, `pull_request_target` runs the default branch's copy of the workflow, so a PR editing that workflow is not exercised by its own run.

+ - In a repository with a workflow dependency lockfile, the run executes the commit the lockfile records, so a `uses:` tag is not mutable there; review the lockfile's changed `commit` entries instead.

+ - In `image:tag@sha256:…` the digest decides and the tag is a label.

+ - `.terraform.lock.hcl` locks providers only; a module floats within its version constraint unless the constraint is exact.

+ - `sensitive = true` hides a Terraform value from CLI output, not from state or plan files, which hold it in plaintext — a plan uploaded as an artifact ships the secret.

+ - Alpine uses musl: a native module compiled in a glibc build stage fails to load in an Alpine runtime stage.

+ - Cleanup that outlasts the grace period never finishes — the platform sends SIGKILL (Kubernetes' default is 30 seconds, `preStop` time included).