devops-code-review

devops-code-review

Validates the full set of pipeline YAML and Dockerfile changes against the merged guidelines before the agent reports completion. Also available on-demand via the /devops-review command for reviewing existing pipelines not generated by the agent.

Pipeline YAML Review Checklist

Run these checks against every pipeline YAML file in scope (generated or existing).

Structure and Stage Ordering

• Pipeline defines at least the required stages: build, dockerBuild, registryPush (or equivalent job names).
• Stage dependency ordering is correct — dockerBuild/registryPush must depend on all upstream scan and test stages.
• No undefined needs: (GH Actions) or dependsOn: (ADO) references — all referenced job/stage names exist.
• Required stages are present and not accidentally disabled.
• No duplicate job/stage names within the same pipeline file.

Runner Configuration

• runs-on (GH Actions) or pool (ADO) is set on every job/stage — no jobs missing a runner declaration.
• Runner labels/pool name matches org-approved configuration from merged guidelines (## Runner Config or ## Pool Config).
• If self-hosted runners are required by org policy, no job uses a GitHub-hosted runner (e.g., ubuntu-latest) except where explicitly approved.
• Resource limits or runner capabilities are set where required by team guidelines.

Action/Task Versioning

• All uses: actions (GH Actions) are pinned to a major version tag or SHA — no unpinned @main, @master, or @latest references.
• All ADO task versions (e.g., Docker@2, SonarQubePrepare@6) are pinned to a major version number.
• Pinned versions match org-approved versions from the merged guidelines or template repo patterns.

Secrets Management

• No hardcoded credentials, tokens, passwords, or registry URLs in any pipeline YAML.
• GitHub Secrets are referenced as ${{ secrets.SECRET_NAME }} (GH Actions) or variable groups (ADO).
• No plaintext secrets in env: blocks, variables: sections, or run: / script: steps.
• Registry login uses approved secret names (ACR_USERNAME, ACR_PASSWORD, SNYK_TOKEN, SONAR_TOKEN, etc.).

Dependency Caching

• Build dependency caching is configured where applicable (npm, Maven, Go modules, pip).
• Cache keys are based on lockfile hashes (e.g., package-lock.json, pom.xml, go.sum, requirements.txt) for deterministic cache invalidation.
• GH Actions: actions/cache@v4 or built-in setup action caching is used.
• ADO: Cache@2 task is used where caching is enabled in config.

Scan Stages

• If sonarScan.enabled is true in config, a SonarQube scan stage/job is present in the pipeline.
• SonarQube step includes fetch-depth: 0 (GH Actions actions/checkout) for full history analysis.
• If snykScan.enabled is true, a Snyk scan stage/job is present.
• Snyk action is language-specific (snyk/actions/dotnet@v1, snyk/actions/node@v1, etc.) not generic.
• If wizScan.enabled is true, a Wiz CLI scan step is present in the docker-build-push job.
• Scan stages that are disabled in config are not present in the output YAML.

Registry Push

• Registry login action/task uses the correct type for the resolved registry (ACR: azure/docker-login@v1 or Docker@2; ECR: aws-actions/amazon-ecr-login@v2 or AWSShellScript@1).
• Image is tagged with a deterministic, immutable tag (commit SHA, build ID) — not latest alone.
• Registry hostname is sourced from an environment variable or secret — not hardcoded.
• Docker build and push steps reference the correct Dockerfile path.

Dockerfile Review Checklist

Run these checks against every Dockerfile in scope.

Multi-Stage Build

• Dockerfile uses multi-stage build (AS build and AS runtime stages minimum).
• Runtime stage does not include build tools, compilers, SDKs, or test frameworks.
• Build artifacts are copied from the build stage using COPY --from=build.

Non-Root User

• A non-root user and group are created in the runtime stage (e.g., addgroup -S appgroup && adduser -S appuser -G appgroup, or groupadd -r/useradd -r).
• USER directive is present in the runtime stage and set to the non-root user.
• USER directive is placed after all RUN installation steps but before ENTRYPOINT/CMD.
• Distroless images: USER nonroot:nonroot is set (UID 65532).

Base Image

• Build stage uses an org-approved build image (Alpine or slim variant as configured).
• Runtime stage uses an org-approved minimal image — no full OS images (ubuntu:latest, debian:latest) unless explicitly approved.
• Base image version is pinned to a specific tag — not latest.
• If defaults.docker.baseImage is set in config, the runtime stage uses that image.

Layer Optimization

• Dependency manifests (e.g., package*.json, pom.xml, go.mod go.sum, requirements.txt) are copied before application source code.
• RUN commands that install packages are grouped into single layers where possible to reduce layer count.
• apt-get or apk add use --no-cache or apt-get clean && rm -rf /var/lib/apt/lists/* to minimize layer size.

Security

• No secrets, credentials, or API keys in any ENV, ARG, or RUN instruction.
• COPY does not reference .env files, credential files, or private keys.
• Build args for secrets use --mount=type=secret or are not persisted in final layers.

Health Check and Labels

• HEALTHCHECK instruction is present in the runtime stage (exception: distroless images — documented absence is acceptable).
• Health check interval (--interval) and timeout (--timeout) are set to reasonable values.
• At least one OCI label (org.opencontainers.image.source) is present in the runtime stage.

Org Compliance Checklist

These checks apply across both pipeline YAML and Dockerfiles and verify adherence to org policy.

• All files comply with every section in the merged guidelines document — each ## section is checked independently.
• The resolved platform (github-actions or azure-devops) is in governance.approvedPlatforms.
• The resolved registry (acr or ecr) is in governance.approvedRegistries.
• The resolved language (dotnet, nodejs, java, golang, python) is in governance.approvedLanguages.
• If ## Forbidden section exists in merged guidelines, all listed forbidden patterns are absent from output files.
• No latest tag in production registry push steps (flagged as blocker if org policy prohibits it).
• Guidelines sections marked as mandatory (if indicated in team guidelines) have zero violations.

YAML LSP Integration

Before reporting review results, run LSP.getDiagnostics on all pipeline YAML files in scope.

Phase	LSP Action
Before review analysis	LSP.getDiagnostics to establish baseline errors in existing files
After checklist review	LSP.getDiagnostics final pass on generated/modified files
During /devops-review command	LSP.getDiagnostics on all files passed to the command

LSP diagnostics surface YAML syntax errors and schema violations (workflow schema for GH Actions, Azure Pipelines schema for ADO) independently of the guideline checklist. LSP errors are always reported as blockers regardless of severity classification in guidelines.

If enforceLsp is true and the LSP is unavailable, the code review step halts with a blocking error rather than proceeding without LSP validation.

Output Format

Each finding is reported with the following structure:

{
  "severity": "blocker | suggestion | nit",
  "file": "<relative file path>",
  "line": "<line number or range, if determinable>",
  "rule": "<exact quoted guideline text, including ## section heading>",
  "message": "<description of the violation>",
  "suggestion": "<recommended correction>",
  "waived": false
}

Severity definitions:

Severity	Meaning	Blocks completion
blocker	Violates a mandatory guideline, org policy, security requirement, or LSP error	Yes — must be resolved before agent reports success
suggestion	Violates a best-practice guideline in a non-mandatory section	No — surfaced for team awareness
nit	Minor style or consistency issue, preference-level feedback	No — surfaced informally

Summary output:

{
  "reviewComplete": true,
  "blockers": 0,
  "suggestions": 2,
  "nits": 1,
  "lspErrors": 0,
  "findings": [],
  "guidelinesChecked": "merged-guidelines (6 sections, 2 layers applied)",
  "filesReviewed": [
    ".github/workflows/ci.yml",
    "Dockerfile"
  ]
}

When blockers is 0 and lspErrors is 0, the review passes and the agent may report completion. When blockers > 0 or lspErrors > 0, the agent must attempt to fix each blocking finding before re-running the review. Suggestions and nits are surfaced in the completion report but do not block.