diff --git a/.github/workflows/security-review-changes.yaml b/.github/workflows/security-review-changes.yaml new file mode 100644 index 00000000..34902f82 --- /dev/null +++ b/.github/workflows/security-review-changes.yaml @@ -0,0 +1,446 @@ +name: Security Review (Changes) + +on: + workflow_dispatch: + inputs: + pull_request_number: + description: "Optional pull request number to review" + required: false + default: "" + agent: + description: "Optional reviewer agent (claude or codex)." + required: false + default: "" + model: + description: "Optional reviewer model override." + required: false + default: "" + timeout_secs: + description: "Optional reviewer timeout in seconds (defaults to 1800)." + required: false + default: "" + force_review: + description: "Force re-review even if already completed for this commit" + required: false + type: boolean + default: false + # pull_request: + # types: + # - opened + # - synchronize + # - reopened + # - ready_for_review + # - labeled + +concurrency: + group: security-review-changes-${{ github.event.pull_request.number || github.run_id }} + cancel-in-progress: false + +jobs: + pr-security-review: + name: Pull Request Security Review + runs-on: ubuntu-24.04 + if: github.event_name == 'pull_request' || github.event_name == 'workflow_dispatch' + permissions: + contents: read + checks: write + + steps: + - name: Checkout repository + uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - name: Install Go + uses: actions/setup-go@v5 + with: + go-version-file: go.mod + + - name: Install Task + uses: arduino/setup-task@v2 + with: + version: 3.x + repo-token: ${{ secrets.GITHUB_TOKEN }} + + - name: Resolve comparison commits + id: revision + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: | + set -euo pipefail + base_sha="${{ github.event.pull_request.base.sha }}" + head_sha="${{ github.sha }}" + pr_number="${{ github.event.pull_request.number }}" + + if [ "${{ github.event_name }}" = "workflow_dispatch" ] && [ -n "${{ github.event.inputs.pull_request_number }}" ]; then + pr_json=$(gh pr view "${{ github.event.inputs.pull_request_number }}" --json baseRefOid,headRefOid,number) + base_sha=$(echo "$pr_json" | jq -r '.baseRefOid') + head_sha=$(echo "$pr_json" | jq -r '.headRefOid') + pr_number=$(echo "$pr_json" | jq -r '.number') + fi + + if [ -n "$base_sha" ]; then + echo "base=$base_sha" >> "$GITHUB_OUTPUT" + fi + if [ -n "$head_sha" ]; then + echo "head=$head_sha" >> "$GITHUB_OUTPUT" + fi + if [ -n "$pr_number" ] && [ "$pr_number" != "null" ]; then + echo "pr=$pr_number" >> "$GITHUB_OUTPUT" + fi + + - name: Collect updated pin targets + id: updatedpins + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: | + set -euo pipefail + base_sha="${{ steps.revision.outputs.base }}" + head_sha="${{ steps.revision.outputs.head }}" + + if [ -z "$base_sha" ] || [ -z "$head_sha" ]; then + echo "has_targets=false" >> "$GITHUB_OUTPUT" + exit 0 + fi + + task ci -- collect-updated-pins \ + --base "$base_sha" \ + --head "$head_sha" \ + --workspace "${{ github.workspace }}" \ + --output-json pins-context.json \ + --summary-md pins-summary.md + + if [ -s pins-context.json ]; then + echo "has_targets=true" >> "$GITHUB_OUTPUT" + echo "context=pins-context.json" >> "$GITHUB_OUTPUT" + else + echo "has_targets=false" >> "$GITHUB_OUTPUT" + fi + + - name: Collect new local servers + id: newservers + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: | + set -euo pipefail + base_sha="${{ steps.revision.outputs.base }}" + head_sha="${{ steps.revision.outputs.head }}" + + if [ -z "$base_sha" ] || [ -z "$head_sha" ]; then + echo "has_targets=false" >> "$GITHUB_OUTPUT" + exit 0 + fi + + task ci -- collect-new-servers \ + --base "$base_sha" \ + --head "$head_sha" \ + --workspace "${{ github.workspace }}" \ + --output-json new-servers-context.json \ + --summary-md new-servers-summary.md + + if [ -s new-servers-context.json ]; then + echo "has_targets=true" >> "$GITHUB_OUTPUT" + echo "context=new-servers-context.json" >> "$GITHUB_OUTPUT" + else + echo "has_targets=false" >> "$GITHUB_OUTPUT" + fi + + - name: Create pending security review checks + if: steps.updatedpins.outputs.has_targets == 'true' || steps.newservers.outputs.has_targets == 'true' + id: checks + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + REVIEW_AGENT_INPUT: ${{ github.event.inputs.agent }} + run: | + set -euo pipefail + + agent="${REVIEW_AGENT_INPUT:-}" + if [ -z "$agent" ]; then + agent="claude" + fi + + head_sha="${{ steps.revision.outputs.head }}" + repo="${{ github.repository }}" + + mkdir -p review-output + + # Store check names and IDs in files for the next step. + > review-output/check-ids.txt + + # Function to create a pending check and return its ID. + create_pending_check() { + local check_name="$1" + local server="$2" + local review_type="$3" + + gh api repos/$repo/check-runs \ + --method POST \ + --field name="$check_name" \ + --field head_sha="$head_sha" \ + --field status="queued" \ + --field output[title]="Security Review: $server" \ + --field output[summary]="Queued for $review_type security review..." \ + --jq '.id' + } + + # Helper to create a slug suitable for check names. + slugify() { + echo "$1" | tr '[:upper:]' '[:lower:]' | sed -E 's/[^a-z0-9]+/-/g' | sed -E 's/^-+|-+$//g' + } + + # Create checks for updated pins (differential reviews). + if [ "${{ steps.updatedpins.outputs.has_targets }}" = "true" ]; then + while read -r target; do + server=$(echo "$target" | jq -r '.server') + project=$(echo "$target" | jq -r '.project') + old_commit=$(echo "$target" | jq -r '.old_commit') + new_commit=$(echo "$target" | jq -r '.new_commit') + + server_slug=$(slugify "$server") + check_name="security-review/$agent/pin/$server_slug" + check_id=$(create_pending_check "$check_name" "$server" "differential") + + echo "$check_name|$check_id|$server|$project|$new_commit|$old_commit|differential" >> review-output/check-ids.txt + done < <(jq -c '.[]' "${{ steps.updatedpins.outputs.context }}") + fi + + # Create checks for new servers (full reviews). + if [ "${{ steps.newservers.outputs.has_targets }}" = "true" ]; then + while read -r target; do + server=$(echo "$target" | jq -r '.server') + project=$(echo "$target" | jq -r '.project') + commit=$(echo "$target" | jq -r '.commit') + + server_slug=$(slugify "$server") + check_name="security-review/$agent/new/$server_slug" + check_id=$(create_pending_check "$check_name" "$server" "full") + + echo "$check_name|$check_id|$server|$project|$commit||full" >> review-output/check-ids.txt + done < <(jq -c '.[]' "${{ steps.newservers.outputs.context }}") + fi + + num_checks=$(wc -l < review-output/check-ids.txt) + echo "Created $num_checks pending check(s)" + echo "has_checks=true" >> "$GITHUB_OUTPUT" + + - name: Run security reviews + if: steps.checks.outputs.has_checks == 'true' + env: + ANTHROPIC_API_KEY: ${{ secrets.ANTHROPIC_API_KEY }} + OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }} + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + REVIEW_AGENT_INPUT: ${{ github.event.inputs.agent }} + REVIEW_MODEL_INPUT: ${{ github.event.inputs.model }} + REVIEW_TIMEOUT_INPUT: ${{ github.event.inputs.timeout_secs }} + FORCE_REVIEW_INPUT: ${{ github.event.inputs.force_review }} + run: | + set -euo pipefail + + agent="${REVIEW_AGENT_INPUT:-}" + if [ -z "$agent" ]; then + agent="claude" + fi + + model="${REVIEW_MODEL_INPUT:-}" + timeout_secs="${REVIEW_TIMEOUT_INPUT:-1800}" + force_review="${FORCE_REVIEW_INPUT:-false}" + + head_sha="${{ steps.revision.outputs.head }}" + repo="${{ github.repository }}" + + # Maximum size for GitHub check output text field (in bytes). + max_check_output_size=65000 + + # Function to determine check conclusion from labels. + determine_conclusion() { + local labels_file="$1" + + if [ ! -s "$labels_file" ]; then + echo "success" + elif grep -qE '^security-blocked$' "$labels_file"; then + echo "failure" + elif grep -qE '^security-risk:(critical|high)$' "$labels_file"; then + echo "failure" + elif grep -qE '^security-risk:medium$' "$labels_file"; then + echo "neutral" + else + echo "success" + fi + } + + # Function to check if a review is already completed (unless forced). + should_skip_review() { + local check_name="$1" + + [ "$force_review" != "true" ] && \ + [ -n "$(gh api repos/$repo/commits/$head_sha/check-runs \ + --jq ".check_runs[] | select(.name == \"$check_name\" and .status == \"completed\")" 2>/dev/null)" ] + } + + # Function to mark a check as skipped. + skip_check() { + local check_id="$1" + local server="$2" + local reason="$3" + + gh api repos/$repo/check-runs/$check_id \ + --method PATCH \ + --field status="completed" \ + --field conclusion="skipped" \ + --field output[title]="Security Review: $server" \ + --field output[summary]="Skipped - $reason" + } + + # Function to create a pending check and return its ID. + create_pending_check() { + local check_name="$1" + local server="$2" + local review_type="$3" + + gh api repos/$repo/check-runs \ + --method POST \ + --field name="$check_name" \ + --field head_sha="$head_sha" \ + --field status="queued" \ + --field output[title]="Security Review: $server" \ + --field output[summary]="Queued for $review_type security review..." \ + --jq '.id' + } + + # Function to run a security review and update a check. + run_review() { + local check_name="$1" + local check_id="$2" + local server="$3" + local project="$4" + local head_commit="$5" + local base_commit="$6" + local review_type="$7" + + if [ -z "$project" ] || [ "$project" = "null" ]; then + echo "Skipping $server: missing project URL." >&2 + skip_check "$check_id" "$server" "missing project URL" + return + fi + if [ -z "$head_commit" ] || [ "$head_commit" = "null" ]; then + echo "Skipping $server: missing head commit." >&2 + skip_check "$check_id" "$server" "missing head commit" + return + fi + + # Check if we should skip this review. + if should_skip_review "$check_name"; then + echo "Skipping $server: review already completed for this commit (use force_review to re-run)" >&2 + skip_check "$check_id" "$server" "review already completed for this commit" + return + fi + + echo "Starting $review_type review for $server..." + + # Mark check as in progress. + gh api repos/$repo/check-runs/$check_id \ + --method PATCH \ + --field status="in_progress" \ + --field output[summary]="Running $review_type security review..." + + # Run the security review. + report_path="review-output/${server}-${review_type}.md" + labels_path="review-output/${server}-${review_type}-labels.txt" + + cmd=(task security-reviewer -- \ + --agent "$agent" \ + --repo "$project" \ + --head "$head_commit" \ + --target-label "$server" \ + --output "$report_path" \ + --labels-output "$labels_path" \ + --timeout "$timeout_secs") + + if [ -n "$model" ]; then + cmd+=(--model "$model") + fi + + if [ "$review_type" = "differential" ]; then + if [ -z "$base_commit" ] || [ "$base_commit" = "null" ]; then + echo "Skipping $server: missing base commit for differential review." >&2 + skip_check "$check_id" "$server" "missing base commit" + return + fi + cmd+=(--base "$base_commit") + fi + + if "${cmd[@]}"; then + # Review succeeded - determine conclusion from labels. + conclusion=$(determine_conclusion "$labels_path") + + # Build summary text. + if [ "$review_type" = "differential" ]; then + summary="Differential review completed (${base_commit:0:7}...${head_commit:0:7})" + else + summary="Full code review completed at ${head_commit:0:7}" + fi + + # Read labels for summary. + if [ -s "$labels_path" ]; then + labels_list=$(cat "$labels_path" | head -5 | paste -sd, -) + summary="${summary}\nLabels: ${labels_list}" + fi + + # Read report and truncate if necessary. + if [ -s "$report_path" ]; then + report_text=$(cat "$report_path") + report_size=${#report_text} + + if [ "$report_size" -gt "$max_check_output_size" ]; then + # Truncate and add notice. + truncate_at=$((max_check_output_size - 200)) + report_text="${report_text:0:$truncate_at}" + report_text="${report_text}\n\n---\n\n**Note:** Report truncated due to size limits. Full report available in workflow artifacts." + fi + else + report_text="No report generated." + fi + + # Update check with results. + gh api repos/$repo/check-runs/$check_id \ + --method PATCH \ + --field status="completed" \ + --field conclusion="$conclusion" \ + --field output[title]="Security Review: $server" \ + --field output[summary]="$summary" \ + --field output[text]="$report_text" + + echo "Completed $review_type review for $server (conclusion: $conclusion)" + else + # Review failed. + gh api repos/$repo/check-runs/$check_id \ + --method PATCH \ + --field status="completed" \ + --field conclusion="failure" \ + --field output[title]="Security Review: $server" \ + --field output[summary]="Security review failed to complete" \ + --field output[text]="The security review process encountered an error. Check workflow logs for details." + + echo "Failed $review_type review for $server" >&2 + fi + } + + # Run all reviews using the check IDs created in the previous step. + while IFS='|' read -r check_name check_id server project head_commit base_commit review_type; do + run_review \ + "$check_name" \ + "$check_id" \ + "$server" \ + "$project" \ + "$head_commit" \ + "$base_commit" \ + "$review_type" || true + done < review-output/check-ids.txt + + - name: Upload security reports + if: always() && (steps.updatedpins.outputs.has_targets == 'true' || steps.newservers.outputs.has_targets == 'true') + uses: actions/upload-artifact@v4 + with: + name: security-reports + path: review-output/ + if-no-files-found: warn diff --git a/.github/workflows/security-review-manual.yaml b/.github/workflows/security-review-manual.yaml new file mode 100644 index 00000000..71260d2d --- /dev/null +++ b/.github/workflows/security-review-manual.yaml @@ -0,0 +1,129 @@ +name: Security Review (Manual) + +on: + workflow_dispatch: + inputs: + servers: + description: "Comma-separated list of local server names to audit (leave blank for all)." + required: false + default: "" + agent: + description: "Optional reviewer agent (claude or codex)." + required: false + default: "" + model: + description: "Optional reviewer model override." + required: false + default: "" + timeout_secs: + description: "Optional reviewer timeout in seconds (defaults to 1800)." + required: false + default: "" + +concurrency: + group: security-review-manual-${{ github.run_id }} + cancel-in-progress: false + +jobs: + full-audit: + name: Execute Full Audit + runs-on: ubuntu-24.04 + permissions: + contents: read + + steps: + - name: Checkout repository + uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - name: Install Go + uses: actions/setup-go@v5 + with: + go-version-file: go.mod + + - name: Install Task + uses: arduino/setup-task@v2 + with: + version: 3.x + repo-token: ${{ secrets.GITHUB_TOKEN }} + + - name: Collect audit targets + id: collect + run: | + set -euo pipefail + task ci -- collect-full-audit \ + --workspace "${{ github.workspace }}" \ + --servers "${{ github.event.inputs.servers }}" \ + --output-json audit-targets.json + + if jq -e '. | length > 0' audit-targets.json >/dev/null; then + echo "has_targets=true" >> "$GITHUB_OUTPUT" + echo "targets=audit-targets.json" >> "$GITHUB_OUTPUT" + else + echo "No audit targets identified; exiting." >&2 + echo "has_targets=false" >> "$GITHUB_OUTPUT" + fi + + - name: Run security reviews + if: steps.collect.outputs.has_targets == 'true' + env: + ANTHROPIC_API_KEY: ${{ secrets.ANTHROPIC_API_KEY }} + OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }} + REVIEW_AGENT_INPUT: ${{ github.event.inputs.agent }} + REVIEW_MODEL_INPUT: ${{ github.event.inputs.model }} + REVIEW_TIMEOUT_INPUT: ${{ github.event.inputs.timeout_secs }} + run: | + set -uo pipefail + agent="${REVIEW_AGENT_INPUT:-}" + if [ -z "$agent" ]; then + agent="claude" + fi + + model="${REVIEW_MODEL_INPUT:-}" + + timeout_secs="${REVIEW_TIMEOUT_INPUT:-1800}" + + mkdir -p reports + + while read -r target; do + server=$(echo "$target" | jq -r '.server') + project=$(echo "$target" | jq -r '.project') + head_commit=$(echo "$target" | jq -r '.commit') + + if [ -z "$project" ] || [ "$project" = "null" ]; then + echo "Skipping $server: missing project URL." >&2 + continue + fi + if [ -z "$head_commit" ] || [ "$head_commit" = "null" ]; then + echo "Skipping $server: missing commit information." >&2 + continue + fi + + report_path="reports/${server}.md" + labels_path="reports/${server}-labels.txt" + cmd=(task security-reviewer -- \ + --agent "$agent" \ + --repo "$project" \ + --head "$head_commit" \ + --target-label "$server" \ + --output "$report_path" \ + --labels-output "$labels_path" \ + --timeout "$timeout_secs") + + if [ -n "$model" ]; then + cmd+=(--model "$model") + fi + + if ! "${cmd[@]}"; then + echo "Security review failed for $server" >&2 + fi + done < <(jq -c '.[]' "${{ steps.collect.outputs.targets }}") + + - name: Upload security reports + if: steps.collect.outputs.has_targets == 'true' + uses: actions/upload-artifact@v4 + with: + name: security-reports + path: reports/ + if-no-files-found: warn diff --git a/.github/workflows/update-pins.yaml b/.github/workflows/update-pins.yaml new file mode 100644 index 00000000..0ac6d84e --- /dev/null +++ b/.github/workflows/update-pins.yaml @@ -0,0 +1,192 @@ +name: Update MCP Server Version Pins + +on: + # schedule: + # - cron: "0 5 * * *" + workflow_dispatch: + inputs: + max_new_prs: + description: "Maximum number of new pull requests to create (leave blank for unlimited)." + required: false + default: "" + servers: + description: "Comma-separated list of servers to update (leave blank for all)." + required: false + default: "" + +concurrency: + group: update-pins + cancel-in-progress: false + +permissions: + contents: write + pull-requests: write + +jobs: + update-pins: + runs-on: ubuntu-24.04 + steps: + - name: Checkout repository + uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - name: Configure Git user + run: | + git config user.name "docker-mcp-bot" + git config user.email "docker-mcp-bot@users.noreply.github.com" + + - name: Install Go + uses: actions/setup-go@v5 + with: + go-version-file: go.mod + + - name: Install Task + uses: arduino/setup-task@v2 + with: + version: 3.x + repo-token: ${{ secrets.GITHUB_TOKEN }} + + - name: Update pinned commits + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: | + task ci -- update-pins + + - name: Collect per-server patches + id: prepare + env: + ALLOWED_SERVERS: ${{ github.event.inputs.servers || '' }} + run: | + # Gather the diff for each modified server YAML and store it as an + # individual patch file so we can open one PR per server. + mkdir -p patches + changed_files=$(git status --porcelain | awk '$2 ~ /^servers\/.*\/server.yaml$/ {print $2}') + if [ -z "$changed_files" ]; then + echo "changed=false" >> "$GITHUB_OUTPUT" + exit 0 + fi + + allowed_servers=$(echo "$ALLOWED_SERVERS" | tr '[:upper:]' '[:lower:]' | tr -d ' ') + server_list=() + for file in $changed_files; do + server=$(basename "$(dirname "$file")") + server_lc=$(echo "$server" | tr '[:upper:]' '[:lower:]') + + if [ -n "$allowed_servers" ]; then + if ! echo ",$allowed_servers," | grep -q ",$server_lc,"; then + continue + fi + fi + + git diff -- "$file" > "patches/${server}.patch" + server_list+=("$server") + done + + if [ ${#server_list[@]} -eq 0 ]; then + echo "No servers matched the provided filter; exiting." >&2 + git checkout -- servers + echo "changed=false" >> "$GITHUB_OUTPUT" + exit 0 + fi + + # Reset the working tree so we can apply patches one-at-a-time. + git checkout -- servers + + # Expose the server list to later steps. + printf '%s\n' "${server_list[@]}" | paste -sd',' - > patches/servers.txt + echo "changed=true" >> "$GITHUB_OUTPUT" + echo "servers=$(cat patches/servers.txt)" >> "$GITHUB_OUTPUT" + + - name: Create or update pull requests + if: steps.prepare.outputs.changed == 'true' + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + MAX_NEW_PRS: ${{ github.event.inputs.max_new_prs || '' }} + run: | + IFS=',' read -ra SERVERS <<< "${{ steps.prepare.outputs.servers }}" + new_pr_limit=$(echo "$MAX_NEW_PRS" | tr -d ' ') + if [ -n "$new_pr_limit" ] && ! [[ "$new_pr_limit" =~ ^[0-9]+$ ]]; then + echo "Invalid max_new_prs value: $new_pr_limit" >&2 + exit 1 + fi + new_pr_count=0 + + for server in "${SERVERS[@]}"; do + patch="patches/${server}.patch" + if [ ! -s "$patch" ]; then + echo "No patch found for $server, skipping." + continue + fi + + # Look up the new commit hash in the patch so we can decide whether + # an existing automation branch already covers it. + new_commit=$(awk '/^\+.*commit:/{print $2}' "$patch" | tail -n1) + branch="automation/update-pin-${server}" + + # Start from a clean copy of main for each server so branches do not + # interfere with one another. + git checkout main + git fetch origin main + git reset --hard origin/main + + # Check if we've hit the new PR limit before doing any work. + branch_exists=false + if git ls-remote --exit-code --heads origin "$branch" >/dev/null 2>&1; then + branch_exists=true + git fetch origin "$branch" + existing_commit=$(git show "origin/${branch}:servers/${server}/server.yaml" 2>/dev/null | awk '/commit:/{print $2}' | tail -n1) + if [ -n "$existing_commit" ] && [ "$existing_commit" = "$new_commit" ]; then + echo "Existing PR for $server already pins ${existing_commit}; skipping." + continue + fi + fi + + # Check PR limit for new branches only. + if [ "$branch_exists" = false ] && [ -n "$new_pr_limit" ] && [ "$new_pr_count" -ge "$new_pr_limit" ]; then + echo "New PR quota reached ($new_pr_limit); skipping $server." + continue + fi + + # Apply the patch onto a fresh branch for this server. + git checkout -B "$branch" origin/main + if ! git apply "$patch"; then + echo "Failed to apply patch for $server, skipping." + continue + fi + + if git diff --quiet; then + echo "No changes after applying patch for $server, skipping." + continue + fi + + # Commit the server YAML change and force-push the automation branch. + git add "servers/${server}/server.yaml" + git commit -m "chore: update pin for ${server}" + if ! git push --force origin "$branch"; then + echo "Failed to push branch for $server, skipping." >&2 + continue + fi + + # Create or update the PR dedicated to this server. + if gh pr view --head "$branch" >/dev/null 2>&1; then + if ! gh pr edit "$branch" \ + --title "chore: update pin for ${server}" \ + --body "Automated commit pin update for ${server}." 2>&1; then + echo "Failed to update PR for $server" >&2 + fi + else + if gh pr create \ + --title "chore: update pin for ${server}" \ + --body "Automated commit pin update for ${server}." \ + --base main \ + --head "$branch" 2>&1; then + new_pr_count=$((new_pr_count + 1)) + else + echo "Failed to create PR for $server" >&2 + fi + fi + done + + # Leave the repository in a clean state. + git checkout main diff --git a/Taskfile.yml b/Taskfile.yml index 6098d96f..06241d36 100644 --- a/Taskfile.yml +++ b/Taskfile.yml @@ -25,6 +25,10 @@ tasks: desc: Clean build artifacts for servers cmd: go run ./cmd/clean {{.CLI_ARGS}} + ci: + desc: Run CI helper utilities + cmd: go run ./cmd/ci {{.CLI_ARGS}} + import: desc: Import a server into the registry cmd: docker mcp catalog import ./catalogs/{{.CLI_ARGS}}/catalog.yaml @@ -38,3 +42,7 @@ tasks: unittest: desc: Run Go unit tests cmd: go test ./... + + security-reviewer: + desc: Run the security reviewer agent + cmd: go run ./cmd/security-reviewer {{.CLI_ARGS}} diff --git a/agents/security-reviewer/Dockerfile b/agents/security-reviewer/Dockerfile new file mode 100644 index 00000000..ba5eb1cd --- /dev/null +++ b/agents/security-reviewer/Dockerfile @@ -0,0 +1,88 @@ +# syntax=docker/dockerfile:1.7 + +# Use a golang image to build the reviewer and proxy binaries (with cgo disabled). +FROM golang:1.25-trixie AS gobuilder +ENV CGO_ENABLED=0 + +# Create and set a working directory for the code. +RUN mkdir -p /security-reviewer +WORKDIR /security-reviewer + +# Copy dependency specifications and ensure they're downloaded. +COPY go.mod go.sum ./ +RUN go mod download + +# Copy reviewer and proxy sources, then build both binaries. +COPY reviewer ./reviewer +COPY proxy ./proxy +RUN mkdir -p /security-reviewer/build +RUN go build -o /security-reviewer/build/reviewer ./reviewer +RUN go build -o /security-reviewer/build/proxy ./proxy + + +# Use a debian image to host the agent. +FROM debian:trixie-slim AS reviewer-image + +# Set up configuration. +ARG CLAUDE_CODE_VERSION=latest +ARG CODEX_CLI_VERSION=latest + +# Install core packages, developer tooling, and runtime dependencies. +ENV DEBIAN_FRONTEND=noninteractive +RUN apt-get update \ + && apt-get install -y --no-install-recommends \ + ca-certificates \ + curl \ + git \ + gnupg \ + jq \ + nodejs \ + npm \ + ripgrep \ + unzip \ + && rm -rf /var/lib/apt/lists/* + +# Install Claude Code and Codex CLIs for headless operation. +RUN npm install --global \ + @anthropic-ai/claude-code@${CLAUDE_CODE_VERSION} \ + @openai/codex@${CODEX_CLI_VERSION} \ + && npm cache clean --force + +# Create a dedicated runtime user and working directories. +RUN useradd --create-home --shell /bin/bash agent \ + && install -d -o agent -g agent /workspace \ + && install -d -o agent -g agent /workspace/input \ + && install -d -o agent -g agent /workspace/output + +# Copy the reviewer entrypoint and assets. +COPY --from=gobuilder /security-reviewer/build/reviewer /opt/security-reviewer/reviewer +COPY --chown=agent:agent prompt-template.md /opt/security-reviewer/prompt-template.md +COPY --chown=agent:agent report-template.md /opt/security-reviewer/report-template.md + +# Set the reviewer user and working directory. +USER agent +WORKDIR /workspace + +# Set the reviewer entrypoint. +ENTRYPOINT ["/opt/security-reviewer/reviewer"] + + +# Use a debian image to host the proxy. +FROM debian:trixie-slim AS proxy-image + +# Install dependencies, including those needed for healthchecks. +ENV DEBIAN_FRONTEND=noninteractive +RUN apt-get update \ + && apt-get install -y --no-install-recommends \ + ca-certificates \ + wget \ + && rm -rf /var/lib/apt/lists/* + +# Copy the proxy entrypoint. +COPY --from=gobuilder /security-reviewer/build/proxy /opt/security-reviewer/proxy + +# Expose the proxy's default port. +EXPOSE 4000 + +# Set the proxy entrypoint. +ENTRYPOINT ["/opt/security-reviewer/proxy"] diff --git a/agents/security-reviewer/compose.yml b/agents/security-reviewer/compose.yml new file mode 100644 index 00000000..a46a7b21 --- /dev/null +++ b/agents/security-reviewer/compose.yml @@ -0,0 +1,72 @@ +services: + reviewer: + build: + context: . + target: reviewer-image + image: security-reviewer:latest + depends_on: + proxy: + condition: service_healthy + environment: + # Configure the reviewer CLIs to authenticate against the local proxy. + # Note: We want Claude Code to use a bearer token when making API requests + # to the proxy, not an X-Api-Key header. If we set ANTHROPIC_API_KEY, it + # will use an X-Api-Key header; ANTHROPIC_AUTH_TOKEN uses a bearer token. + ANTHROPIC_AUTH_TOKEN: "sk-compose-clients" + ANTHROPIC_BASE_URL: "http://proxy:4000/anthropic" + CLAUDE_REVIEW_MODEL: ${CLAUDE_REVIEW_MODEL:-claude-sonnet-4-5-20250929} + # Note: Codex won't respect the OPENAI_API_KEY environment variable when + # running in non-interactive mode, it will only use the CODEX_API_KEY + # environment variable. + CODEX_API_KEY: "sk-compose-clients" + OPENAI_BASE_URL: "http://proxy:4000/openai" + CODEX_REVIEW_MODEL: ${CODEX_REVIEW_MODEL:-gpt-5-codex} + REVIEW_AGENT: ${REVIEW_AGENT:-claude} + REVIEW_HEAD_SHA: ${REVIEW_HEAD_SHA:-} + REVIEW_BASE_SHA: ${REVIEW_BASE_SHA:-} + REVIEW_TARGET_LABEL: ${REVIEW_TARGET_LABEL:-} + REVIEW_AGENT_EXTRA_ARGS: ${REVIEW_AGENT_EXTRA_ARGS:-} + REVIEW_TIMEOUT_SECS: ${REVIEW_TIMEOUT_SECS:-3600} + volumes: + - type: bind + source: ${REVIEW_REPOSITORY_PATH:?Set REVIEW_REPOSITORY_PATH to bind the repository under review} + target: /workspace/input/repository + read_only: true + - type: bind + source: ${REVIEW_OUTPUT_PATH:?Set REVIEW_OUTPUT_PATH for report output} + target: /workspace/output + networks: + - internal + + proxy: + build: + context: . + target: proxy-image + image: security-reviewer-proxy:latest + environment: + PROXY_LISTEN_ADDR: ":4000" + PROXY_OPENAI_BASE_URL: ${PROXY_OPENAI_BASE_URL:-https://api.openai.com/v1} + # NOTE: Anthropic clients such as Claude Code will implicitly include a + # /v1 component in their request URLs, so if you override this environment + # variable, be sure not to include a /v1 path component. + PROXY_ANTHROPIC_BASE_URL: ${PROXY_ANTHROPIC_BASE_URL:-https://api.anthropic.com} + PROXY_API_KEY: "sk-compose-clients" + OPENAI_API_KEY: ${OPENAI_API_KEY:-} + ANTHROPIC_API_KEY: ${ANTHROPIC_API_KEY:-} + healthcheck: + test: ["CMD-SHELL", "wget --no-verbose --tries=1 http://localhost:4000/health/liveness || exit 1"] + interval: 5s + timeout: 5s + retries: 5 + start_period: 5s + networks: + - internal + - external + restart: unless-stopped + +networks: + internal: + driver: bridge + internal: true + external: + driver: bridge diff --git a/agents/security-reviewer/go.mod b/agents/security-reviewer/go.mod new file mode 100644 index 00000000..f48fd52a --- /dev/null +++ b/agents/security-reviewer/go.mod @@ -0,0 +1,5 @@ +module github.com/docker/mcp-registry/agents/security-reviewer + +go 1.25.3 + +require github.com/mattn/go-shellwords v1.0.12 diff --git a/agents/security-reviewer/go.sum b/agents/security-reviewer/go.sum new file mode 100644 index 00000000..da8b56c3 --- /dev/null +++ b/agents/security-reviewer/go.sum @@ -0,0 +1,2 @@ +github.com/mattn/go-shellwords v1.0.12 h1:M2zGm7EW6UQJvDeQxo4T51eKPurbeFbe8WtebGE2xrk= +github.com/mattn/go-shellwords v1.0.12/go.mod h1:EZzvwXDESEeg03EKmM+RmDnNOPKG4lLtQsUlTZDWQ8Y= diff --git a/agents/security-reviewer/prompt-template.md b/agents/security-reviewer/prompt-template.md new file mode 100644 index 00000000..ee46660b --- /dev/null +++ b/agents/security-reviewer/prompt-template.md @@ -0,0 +1,44 @@ +# Docker MCP Security Review Instructions + +$MODE_SUMMARY + +Security review metadata: +- Mode: $MODE_LABEL +- Repository name: $TARGET_LABEL +- Repository path: $REPOSITORY_PATH +- Head commit: $HEAD_COMMIT +- Base commit: $BASE_COMMIT +- Commit range: $COMMIT_RANGE + +Core analysis directives: +- $GIT_DIFF_HINT +- Hunt aggressively for intentionally malicious behavior (exfiltration, + persistence, destructive payloads) in addition to accidental security bugs. +- Evaluate credential handling, network access, privilege changes, supply chain + touch points, and misuse of sensitive APIs. +- Use only the tools provided (git, ripgrep, jq, etc.); outbound network access + is unavailable. +- Keep any files you create within /workspace or $REPOSITORY_PATH. + +Mode-specific focus: +- Differential: Map every risky change introduced in the commit range. Call out + suspicious files, shell commands, or configuration shifts. Note beneficial + security hardening too. +- Full: Examine the entire repository snapshot. Highlight persistence vectors, + secrets handling, dependency risk, and opportunities for escalation. + +Report expectations: +- Reproduce every heading, section order, and field exactly as written in + `$REPORT_TEMPLATE_PATH`; replace bracketed placeholders with concrete + content but do not add or remove sections. +- Save the final report to $REPORT_PATH. +- Articulate severity, impact, evidence, and remediation for each issue. + +Labeling guidance: +- Write labels to $LABELS_PATH, one per line. +- Emit exactly one overall risk label in the form `security-risk:` where + `` is one of `critical`, `high`, `medium`, `low`, or `info`. +- Align the chosen label with the overall risk level declared in the report. +- If you identify blocking or critical issues that must halt release, also + include the label `security-blocked` on a separate line. +- Leave $LABELS_PATH empty only if the review cannot be completed. diff --git a/agents/security-reviewer/proxy/main.go b/agents/security-reviewer/proxy/main.go new file mode 100644 index 00000000..a1efe642 --- /dev/null +++ b/agents/security-reviewer/proxy/main.go @@ -0,0 +1,337 @@ +package main + +import ( + "context" + "crypto/subtle" + "errors" + "fmt" + "log" + "net" + "net/http" + "net/http/httputil" + "net/url" + "os" + "os/signal" + "strings" + "syscall" + "time" +) + +const ( + // defaultListenAddr is the fallback bind address when none is provided via PROXY_LISTEN_ADDR. + defaultListenAddr = ":4000" + // defaultOpenAIBaseURL is the upstream OpenAI API base path used when none is provided. + defaultOpenAIBaseURL = "https://api.openai.com/v1" + // defaultAnthropicBaseURL is the upstream Anthropic API base path used when none is provided. + // NOTE: Anthropic clients are expected to include /v1 in their requests, so + // it is not idiomatic to include it in the base URL. + defaultAnthropicBaseURL = "https://api.anthropic.com" + // openAIInboundPrefix is the path prefix used to route requests to OpenAI. + openAIInboundPrefix = "/openai/" + // anthropicInboundPrefix is the path prefix used to route requests to Anthropic. + anthropicInboundPrefix = "/anthropic/" + // healthPath is the HTTP endpoint used for container health checks. + healthPath = "/health/liveness" + // headerAuthorization is the inbound HTTP header that carries bearer tokens. + headerAuthorization = "Authorization" + // headerAnthropicAPIKey is the Anthropic-specific header carrying API keys. + headerAnthropicAPIKey = "X-Api-Key" +) + +// providerProxy defines how to forward requests to a specific upstream API. +type providerProxy struct { + // Prefix is the inbound path prefix handled by the provider. + Prefix string + // Target is the upstream endpoint used to service requests for the provider. + Target *url.URL + // HeaderName is the outbound header carrying the provider-specific credential. + HeaderName string + // HeaderValue is the credential value set on outbound requests. + HeaderValue string + // DisplayName is the human-readable name of the provider used in logs. + DisplayName string +} + +// main configures the proxy service and starts the HTTP server. +func main() { + cfg, err := loadConfig() + if err != nil { + log.Fatalf("proxy configuration error: %v", err) + } + + ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGINT, syscall.SIGTERM) + defer stop() + + mux := http.NewServeMux() + mux.HandleFunc(healthPath, handleHealth) + + mountProxy(mux, cfg.openAIProxy, cfg.clientToken) + mountProxy(mux, cfg.anthropicProxy, cfg.clientToken) + mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { + http.NotFound(w, r) + }) + + server := &http.Server{ + Addr: cfg.listenAddr, + Handler: withLogging(mux), + ReadTimeout: 15 * time.Second, + // WriteTimeout needs to be relatively high because it limits how long + // the upstream inference API has to respond. + WriteTimeout: 3600 * time.Second, + IdleTimeout: 60 * time.Second, + } + + log.Printf("proxy listening on %s (OpenAI -> %s, Anthropic -> %s)", + cfg.listenAddr, cfg.openAIProxy.Target.String(), cfg.anthropicProxy.Target.String()) + + go func() { + <-ctx.Done() + shutdownCtx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + if err := server.Shutdown(shutdownCtx); err != nil { + log.Printf("proxy shutdown error: %v", err) + } + }() + + if err = server.ListenAndServe(); err != nil && !errors.Is(err, http.ErrServerClosed) { + log.Fatalf("proxy server error: %v", err) + } +} + +// proxyConfig captures runtime settings for the reverse proxy. +type proxyConfig struct { + listenAddr string + openAIProxy providerProxy + anthropicProxy providerProxy + clientToken string +} + +// loadConfig reads environment variables and constructs the proxy configuration. +func loadConfig() (proxyConfig, error) { + listen := firstNonEmpty(os.Getenv("PROXY_LISTEN_ADDR"), defaultListenAddr) + + clientToken := strings.TrimSpace(os.Getenv("PROXY_API_KEY")) + if clientToken == "" { + return proxyConfig{}, errors.New("PROXY_API_KEY must be set") + } + + openAIBase, err := parseBaseURL(firstNonEmpty(os.Getenv("PROXY_OPENAI_BASE_URL"), defaultOpenAIBaseURL)) + if err != nil { + return proxyConfig{}, fmt.Errorf("parse OpenAI base URL: %w", err) + } + anthropicBase, err := parseBaseURL(firstNonEmpty(os.Getenv("PROXY_ANTHROPIC_BASE_URL"), defaultAnthropicBaseURL)) + if err != nil { + return proxyConfig{}, fmt.Errorf("parse Anthropic base URL: %w", err) + } + + openAIKey := strings.TrimSpace(os.Getenv("OPENAI_API_KEY")) + anthropicKey := strings.TrimSpace(os.Getenv("ANTHROPIC_API_KEY")) + + openAIProxy := providerProxy{ + Prefix: openAIInboundPrefix, + Target: openAIBase, + HeaderName: headerAuthorization, + HeaderValue: bearerValue(openAIKey), + DisplayName: "OpenAI", + } + anthropicProxy := providerProxy{ + Prefix: anthropicInboundPrefix, + Target: anthropicBase, + HeaderName: headerAnthropicAPIKey, + HeaderValue: anthropicKey, + DisplayName: "Anthropic", + } + + return proxyConfig{ + listenAddr: listen, + openAIProxy: openAIProxy, + anthropicProxy: anthropicProxy, + clientToken: clientToken, + }, nil +} + +// mountProxy attaches a provider proxy to the HTTP mux. +func mountProxy(mux *http.ServeMux, provider providerProxy, clientToken string) { + handler := buildProviderHandler(provider, clientToken) + mux.Handle(provider.Prefix, handler) +} + +// buildProviderHandler creates an HTTP handler that forwards requests to the provider. +func buildProviderHandler(provider providerProxy, clientToken string) http.Handler { + proxy := httputil.NewSingleHostReverseProxy(provider.Target) + originalDirector := proxy.Director + proxy.Director = func(req *http.Request) { + inboundPath := req.URL.Path + inboundRawPath := req.URL.RawPath + originalDirector(req) + rewriteRequest(req, inboundPath, inboundRawPath, provider) + } + proxy.ErrorHandler = func(w http.ResponseWriter, r *http.Request, err error) { + log.Printf("proxy error [%s]: %v", provider.DisplayName, err) + http.Error(w, "upstream request failed", http.StatusBadGateway) + } + + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if !strings.HasPrefix(r.URL.Path, provider.Prefix) { + http.NotFound(w, r) + return + } + if provider.HeaderValue == "" { + log.Printf("proxy warning [%s]: request rejected due to missing API key", provider.DisplayName) + http.Error(w, "upstream API key is not configured", http.StatusServiceUnavailable) + return + } + if !validateClientToken(r.Header.Get(headerAuthorization), clientToken) { + log.Printf("proxy warning [%s]: request rejected due to missing or invalid client bearer token", provider.DisplayName) + http.Error(w, "invalid bearer token", http.StatusUnauthorized) + return + } + + proxy.ServeHTTP(w, r) + }) +} + +// rewriteRequest adjusts the outbound request before it is sent upstream. +func rewriteRequest(req *http.Request, inboundPath, inboundRawPath string, provider providerProxy) { + req.URL.Scheme = provider.Target.Scheme + req.URL.Host = provider.Target.Host + req.Host = provider.Target.Host + + trimmedPath := strings.TrimPrefix(inboundPath, provider.Prefix) + if trimmedPath == inboundPath { + trimmedPath = "" + } + + basePath := provider.Target.Path + extraPath := singleLeadingSlash(trimmedPath) + req.URL.Path = joinURLPath(basePath, extraPath) + + trimmedRaw := "" + if inboundRawPath != "" { + trimmedRaw = strings.TrimPrefix(inboundRawPath, provider.Prefix) + if trimmedRaw == inboundRawPath { + trimmedRaw = "" + } + } + if trimmedRaw != "" { + req.URL.RawPath = joinURLPath(basePath, singleLeadingSlash(trimmedRaw)) + } else { + req.URL.RawPath = req.URL.Path + } + + stripSensitiveHeaders(req.Header) + + if provider.HeaderName == headerAuthorization { + req.Header.Set(headerAuthorization, provider.HeaderValue) + } else if provider.HeaderName != "" { + req.Header.Set(provider.HeaderName, provider.HeaderValue) + } +} + +// stripSensitiveHeaders removes inbound authentication headers that should not propagate upstream. +func stripSensitiveHeaders(header http.Header) { + header.Del(headerAuthorization) + header.Del(headerAnthropicAPIKey) +} + +// joinURLPath concatenates base and additional path segments. +func joinURLPath(basePath, extraPath string) string { + switch { + case basePath == "" || basePath == "/": + return singleLeadingSlash(extraPath) + case extraPath == "" || extraPath == "/": + return singleLeadingSlash(basePath) + default: + return singleLeadingSlash(strings.TrimSuffix(basePath, "/") + "/" + strings.TrimPrefix(extraPath, "/")) + } +} + +// singleLeadingSlash ensures the provided path has a leading slash. +func singleLeadingSlash(path string) string { + if path == "" { + return "/" + } + if !strings.HasPrefix(path, "/") { + return "/" + path + } + return path +} + +// withLogging wraps the handler with structured request logging. +func withLogging(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + start := time.Now() + next.ServeHTTP(w, r) + duration := time.Since(start) + remote := remoteAddr(r.Context(), r.RemoteAddr) + if r.URL.Path != healthPath { + log.Printf("proxy request method=%s path=%s remote=%s duration=%s", + r.Method, r.URL.Path, remote, duration) + } + }) +} + +// remoteAddr normalizes the remote address for logging. +func remoteAddr(ctx context.Context, fallback string) string { + if peer, ok := ctx.Value(http.LocalAddrContextKey).(net.Addr); ok { + return peer.String() + } + return fallback +} + +// handleHealth responds to health check requests. +func handleHealth(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "text/plain") + _, _ = w.Write([]byte("ok")) +} + +// parseBaseURL validates and normalizes the upstream base URL. +func parseBaseURL(raw string) (*url.URL, error) { + parsed, err := url.Parse(raw) + if err != nil { + return nil, err + } + if parsed.Scheme == "" || parsed.Host == "" { + return nil, fmt.Errorf("invalid URL %q (must include scheme and host)", raw) + } + if !strings.HasSuffix(parsed.Path, "/") { + parsed.Path += "/" + } + return parsed, nil +} + +// bearerValue formats the bearer token header. +func bearerValue(key string) string { + key = strings.TrimSpace(key) + if key == "" { + return "" + } + return "Bearer " + key +} + +// firstNonEmpty returns the first non-empty string in candidates. +func firstNonEmpty(values ...string) string { + for _, v := range values { + if strings.TrimSpace(v) != "" { + return v + } + } + return "" +} + +// validateClientToken ensures inbound requests present the proxy bearer secret using +// a constant-time comparison to avoid leaking timing information. +func validateClientToken(headerValue, expectedToken string) bool { + if expectedToken == "" { + return false + } + parts := strings.SplitN(headerValue, " ", 2) + if len(parts) != 2 { + return false + } + if !strings.EqualFold(parts[0], "bearer") { + return false + } + provided := strings.TrimSpace(parts[1]) + return subtle.ConstantTimeCompare([]byte(provided), []byte(expectedToken)) == 1 +} diff --git a/agents/security-reviewer/report-template.md b/agents/security-reviewer/report-template.md new file mode 100644 index 00000000..f2be42fe --- /dev/null +++ b/agents/security-reviewer/report-template.md @@ -0,0 +1,37 @@ +# Security Review Report + +## Scope Summary +- **Review Mode:** [Differential/Full] +- **Repository:** [URL or path] +- **Head Commit:** [SHA] +- **Base Commit:** [SHA or N/A] +- **Commit Range:** [base...head or single commit] +- **Overall Risk Level:** [CRITICAL/HIGH/MEDIUM/LOW/INFO] + +## Executive Summary +[One to two paragraphs summarizing the repository's security posture.] + +## Detailed Findings +### [Finding title — severity] +- **Impact:** [Describe the risk to users or systems.] +- **Evidence:** [Point to files, commits, or code snippets.] +- **Recommendation:** [Explain how to mitigate or remediate.] + +### [Finding title — severity] +- **Impact:** [...] +- **Evidence:** [...] +- **Recommendation:** [...] + +## Defense-In-Depth Observations +- [Optional notes about positive security patterns or remaining gaps.] + +## Recommended Labels +- [Label name — justification] +- [Label name — justification] + +## Next Steps +- [Follow-up task or owner] +- [Timeline or priority guidance] + +## Conclusion +[Final remarks and call-outs for reviewers.] diff --git a/agents/security-reviewer/reviewer/agent.go b/agents/security-reviewer/reviewer/agent.go new file mode 100644 index 00000000..77ccd611 --- /dev/null +++ b/agents/security-reviewer/reviewer/agent.go @@ -0,0 +1,35 @@ +package main + +import ( + "context" + "fmt" + "os/exec" +) + +const ( + // agentNameClaude identifies the Claude Code based reviewer. + agentNameClaude = "claude" + // agentNameCodex identifies the Codex based reviewer. + agentNameCodex = "codex" +) + +// reviewerAgent defines the behavior required by each agent implementation. +type reviewerAgent interface { + Name() string + // ModelEnvVar returns the environment variable that overrides the agent's model, or empty when not applicable. + ModelEnvVar() string + // BuildCommand returns the configured command used to invoke the agent. + BuildCommand(ctx context.Context, inv agentInvocation) (*exec.Cmd, error) +} + +// selectAgent resolves an agent by name. +func selectAgent(name string) (reviewerAgent, error) { + switch name { + case agentNameClaude: + return claudeAgent{}, nil + case agentNameCodex: + return codexAgent{}, nil + default: + return nil, fmt.Errorf("unsupported review agent: %s", name) + } +} diff --git a/agents/security-reviewer/reviewer/claude.go b/agents/security-reviewer/reviewer/claude.go new file mode 100644 index 00000000..2b4a66da --- /dev/null +++ b/agents/security-reviewer/reviewer/claude.go @@ -0,0 +1,59 @@ +package main + +import ( + "context" + "fmt" + "os/exec" + "strings" + + "github.com/mattn/go-shellwords" +) + +// claudeAgent implements reviewerAgent for Claude Code. +type claudeAgent struct{} + +// Name returns the stable identifier for the Claude agent implementation. +func (claudeAgent) Name() string { + return agentNameClaude +} + +// ModelEnvVar exposes the environment variable used to override the model. +func (claudeAgent) ModelEnvVar() string { + // Claude Code reads its target model from CLAUDE_REVIEW_MODEL. + return "CLAUDE_REVIEW_MODEL" +} + +// BuildCommand constructs the Claude CLI invocation for a review run. +func (claudeAgent) BuildCommand(ctx context.Context, inv agentInvocation) (*exec.Cmd, error) { + // When running Claude Code in non-interactive mode, the only output format + // that gives regular progress updates is stream-json - anything else waits + // for the full analysis to complete and then provides all the output at + // once. It would be nice if Claude Code had something like a stream-text + // mode, and there's a request for that here: + // https://github.com/anthropics/claude-code/issues/4346 + // In the meantime, I think we'll just live with the JSON output, since at + // least that gives some indication of progress and what's happening. + args := []string{ + "--print", "--verbose", + "--output-format", "stream-json", + "--dangerously-skip-permissions", + } + if strings.TrimSpace(inv.Model) != "" { + args = append(args, "--model", inv.Model) + } + if strings.TrimSpace(inv.ExtraArgs) != "" { + parsed, err := shellwords.Parse(inv.ExtraArgs) + if err != nil { + return nil, fmt.Errorf("parse extra args: %w", err) + } + args = append(args, parsed...) + } + + cmd := exec.CommandContext(ctx, "claude", args...) + cmd.Stdin = strings.NewReader(inv.Prompt) + if inv.WorkingDir != "" { + cmd.Dir = inv.WorkingDir + } + + return cmd, nil +} diff --git a/agents/security-reviewer/reviewer/codex.go b/agents/security-reviewer/reviewer/codex.go new file mode 100644 index 00000000..7aac74b7 --- /dev/null +++ b/agents/security-reviewer/reviewer/codex.go @@ -0,0 +1,52 @@ +package main + +import ( + "context" + "fmt" + "os/exec" + "strings" + + "github.com/mattn/go-shellwords" +) + +// codexAgent implements reviewerAgent for the OpenAI Codex CLI. +type codexAgent struct{} + +// Name returns the stable identifier for the Codex agent implementation. +func (codexAgent) Name() string { + return agentNameCodex +} + +// ModelEnvVar exposes the environment variable used to override Codex models. +func (codexAgent) ModelEnvVar() string { + // Codex shells read from CODEX_REVIEW_MODEL when provided. + return "CODEX_REVIEW_MODEL" +} + +// BuildCommand constructs the Codex CLI invocation for a review run. +func (codexAgent) BuildCommand(ctx context.Context, inv agentInvocation) (*exec.Cmd, error) { + args := []string{ + "exec", + "--skip-git-repo-check", + "--dangerously-bypass-approvals-and-sandbox", + } + if strings.TrimSpace(inv.Model) != "" { + args = append(args, "--model", inv.Model) + } + if strings.TrimSpace(inv.ExtraArgs) != "" { + parsed, err := shellwords.Parse(inv.ExtraArgs) + if err != nil { + return nil, fmt.Errorf("parse extra args: %w", err) + } + args = append(args, parsed...) + } + args = append(args, "-") + + cmd := exec.CommandContext(ctx, "codex", args...) + cmd.Stdin = strings.NewReader(inv.Prompt) + if inv.WorkingDir != "" { + cmd.Dir = inv.WorkingDir + } + + return cmd, nil +} diff --git a/agents/security-reviewer/reviewer/main.go b/agents/security-reviewer/reviewer/main.go new file mode 100644 index 00000000..72ea74e3 --- /dev/null +++ b/agents/security-reviewer/reviewer/main.go @@ -0,0 +1,392 @@ +package main + +import ( + "context" + "errors" + "fmt" + "io/fs" + "os" + "os/signal" + "path/filepath" + "strconv" + "strings" + "syscall" + "time" +) + +const ( + // promptTemplatePath is the location of the static prompt template bundled with the image. + promptTemplatePath = "/opt/security-reviewer/prompt-template.md" + // reportTemplatePath is the location of the report template referenced in prompts. + reportTemplatePath = "/opt/security-reviewer/report-template.md" + // defaultPromptPath is where the rendered prompt is written for the agent to consume. + defaultPromptPath = "/workspace/input/prompt.md" + // defaultRepositoryPath is the mount point containing the repository under review. + defaultRepositoryPath = "/workspace/input/repository" + // defaultReportPath is the expected location for the agent's security report. + defaultReportPath = "/workspace/output/report.md" + // defaultLabelsPath is the expected location for the agent's label output. + defaultLabelsPath = "/workspace/output/labels.txt" + // defaultReviewAgent is the reviewer implementation used when none is specified. + defaultReviewAgent = "claude" + // defaultAgentWorkingDir is the directory from which the agent executes. + defaultAgentWorkingDir = "/workspace" + // defaultTimeout bounds how long the reviewer will wait for the agent to complete. + defaultTimeout = time.Hour + + // envReviewAgent selects which reviewer agent to run. + envReviewAgent = "REVIEW_AGENT" + // envAgentExtraArgs contains optional CLI arguments passed through to the agent. + envAgentExtraArgs = "REVIEW_AGENT_EXTRA_ARGS" + // envReviewHeadSHA provides the head commit SHA under review. + envReviewHeadSHA = "REVIEW_HEAD_SHA" + // envReviewBaseSHA provides the base commit SHA when performing differential reviews. + envReviewBaseSHA = "REVIEW_BASE_SHA" + // envReviewTarget supplies a human-readable label describing the review target. + envReviewTarget = "REVIEW_TARGET_LABEL" + // envReviewTimeout allows callers to override the agent execution timeout in seconds. + envReviewTimeout = "REVIEW_TIMEOUT_SECS" +) + +// ReviewMode enumerates supported security review modes. +type ReviewMode string + +const ( + // ReviewModeFull requests a full repository audit. + ReviewModeFull ReviewMode = "full" + // ReviewModeDifferential requests a differential review between two commits. + ReviewModeDifferential ReviewMode = "differential" +) + +// agentInvocation captures execution hints per reviewer agent. +type agentInvocation struct { + // Prompt is the rendered instruction text passed over stdin. + Prompt string + // Model identifies the model to invoke, when the agent supports overrides. + Model string + // ExtraArgs contains caller-supplied CLI arguments for the agent. + ExtraArgs string + // WorkingDir specifies the directory where the agent command executes. + WorkingDir string +} + +// promptPlaceholders stores values substituted into the static prompt template. +type promptPlaceholders struct { + // ModeLabel is the human friendly descriptor for the review mode. + ModeLabel string + // ModeSummary highlights the responsibilities for the current mode. + ModeSummary string + // TargetLabel is an identifier referencing the repository under review. + TargetLabel string + // RepositoryPath points to the checked-out repository mount. + RepositoryPath string + // HeadCommit is the commit under audit. + HeadCommit string + // BaseCommit is the comparison commit for diff reviews. + BaseCommit string + // CommitRange renders the ... spec for diff reviews. + CommitRange string + // GitDiffHint guides the agent on how to inspect the change set. + GitDiffHint string + // ReportPath denotes where the agent should write its report. + ReportPath string + // LabelsPath denotes where the agent should write labels for automation. + LabelsPath string + // ReportTemplatePath tells the agent which template to follow exactly. + ReportTemplatePath string +} + +// main configures logging, resolves environment, and runs the selected agent. +func main() { + ctx, stop := signal.NotifyContext(context.Background(), syscall.SIGINT, syscall.SIGTERM) + defer stop() + + // Run the review workflow and exit non-zero on failure so the container signals an error. + if err := run(ctx); err != nil { + logError(err) + os.Exit(1) + } +} + +// run orchestrates prompt generation and agent execution. +func run(ctx context.Context) error { + // Parse review configuration from the environment. + headSHA, err := fetchEnv(envReviewHeadSHA, true) + if err != nil { + return err + } + baseSHA, err := fetchEnv(envReviewBaseSHA, false) + if err != nil { + return err + } + + mode := ReviewModeFull + if baseSHA != "" { + mode = ReviewModeDifferential + } + + targetLabel, err := fetchEnv(envReviewTarget, false) + if err != nil { + return err + } + + // Ensure the repository mount is present before processing. + if err = ensureDirectory(defaultRepositoryPath); err != nil { + return err + } + + promptContent, err := buildPromptContent(mode, targetLabel, headSHA, baseSHA) + if err != nil { + return err + } + if err = ensureParent(defaultPromptPath); err != nil { + return err + } + if err = os.WriteFile(defaultPromptPath, []byte(promptContent), 0o644); err != nil { + return fmt.Errorf("write prompt: %w", err) + } + logInfo(fmt.Sprintf("Rendered prompt to %s.", defaultPromptPath)) + + // Select the reviewer implementation and build invocation parameters. + agentName, err := fetchEnv(envReviewAgent, false) + if err != nil { + return err + } + if agentName == "" { + agentName = defaultReviewAgent + } + agentKey := strings.ToLower(strings.TrimSpace(agentName)) + agent, err := selectAgent(agentKey) + if err != nil { + return err + } + + var model string + if envName := agent.ModelEnvVar(); envName != "" { + model, err = fetchEnv(envName, false) + if err != nil { + return err + } + } + + extraArgs, _ := fetchEnv(envAgentExtraArgs, false) + inv := agentInvocation{ + Prompt: promptContent, + Model: model, + ExtraArgs: extraArgs, + WorkingDir: defaultAgentWorkingDir, + } + + timeout, err := resolveTimeout() + if err != nil { + return err + } + + agentCtx, cancel := context.WithTimeout(ctx, timeout) + defer cancel() + + if mode == ReviewModeDifferential { + logInfo(fmt.Sprintf( + "Starting differential review (agent=%s head=%s base=%s label=%s).", + agent.Name(), headSHA, baseSHA, targetLabel, + )) + } else { + logInfo(fmt.Sprintf( + "Starting full review (agent=%s head=%s label=%s).", + agent.Name(), headSHA, targetLabel, + )) + } + + // Execute the agent command and relay its output streams. + if err := runAgent(agentCtx, agent, inv); err != nil { + return err + } + + // Inspect the outputs and warn if they were not produced. + if fileExists(defaultReportPath) { + logInfo(fmt.Sprintf("Report stored at %s.", defaultReportPath)) + } else { + logWarn(fmt.Sprintf("Report not produced at %s.", defaultReportPath)) + } + if fileExists(defaultLabelsPath) { + logInfo(fmt.Sprintf("Labels stored at %s.", defaultLabelsPath)) + } else { + logWarn(fmt.Sprintf("Labels not produced at %s.", defaultLabelsPath)) + } + + logInfo("Security review completed successfully.") + return nil +} + +// fetchEnv reads an environment variable and validates presence when required. +func fetchEnv(name string, required bool) (string, error) { + value := strings.TrimSpace(os.Getenv(name)) + if value == "" && required { + return "", fmt.Errorf("missing required environment variable: %s", name) + } + return value, nil +} + +// ensureParent creates directories needed for the provided path. +func ensureParent(path string) error { + dir := filepath.Dir(path) + if dir == "." || dir == "" { + return nil + } + return os.MkdirAll(dir, 0o755) +} + +// runAgent executes the reviewer agent command and captures output streams. +func runAgent(ctx context.Context, agent reviewerAgent, inv agentInvocation) error { + cmd, err := agent.BuildCommand(ctx, inv) + if err != nil { + return err + } + + cmd.Stdout = os.Stdout + cmd.Stderr = os.Stderr + + if err = cmd.Run(); err != nil { + return fmt.Errorf("%s invocation failed: %w", agent.Name(), err) + } + return nil +} + +func resolveTimeout() (time.Duration, error) { + value := strings.TrimSpace(os.Getenv(envReviewTimeout)) + if value == "" { + return defaultTimeout, nil + } + secs, err := strconv.Atoi(value) + if err != nil || secs <= 0 { + return 0, fmt.Errorf("invalid %s value %q", envReviewTimeout, value) + } + return time.Duration(secs) * time.Second, nil +} + +// buildPromptContent renders a concrete prompt for the selected review mode. +func buildPromptContent(mode ReviewMode, targetLabel, headSHA, baseSHA string) (string, error) { + displayLabel := strings.TrimSpace(targetLabel) + if displayLabel == "" { + displayLabel = "[Not provided]" + } + baseDisplay := "[Not applicable]" + commitRange := "[Not applicable]" + if mode == ReviewModeDifferential { + baseDisplay = baseSHA + commitRange = fmt.Sprintf("%s...%s", baseSHA, headSHA) + } + + ph := promptPlaceholders{ + ModeLabel: modeLabel(mode), + ModeSummary: modeSummary(mode), + TargetLabel: displayLabel, + RepositoryPath: defaultRepositoryPath, + HeadCommit: headSHA, + BaseCommit: baseDisplay, + CommitRange: commitRange, + GitDiffHint: gitDiffHint(mode, baseSHA, headSHA), + ReportPath: defaultReportPath, + LabelsPath: defaultLabelsPath, + ReportTemplatePath: reportTemplatePath, + } + return renderPrompt(ph) +} + +// renderPrompt injects placeholder values into the prompt template. +func renderPrompt(ph promptPlaceholders) (string, error) { + templateBytes, err := os.ReadFile(promptTemplatePath) + if err != nil { + return "", fmt.Errorf("read prompt template: %w", err) + } + replacer := strings.NewReplacer( + "$MODE_LABEL", ph.ModeLabel, + "$MODE_SUMMARY", ph.ModeSummary, + "$TARGET_LABEL", ph.TargetLabel, + "$REPOSITORY_PATH", ph.RepositoryPath, + "$HEAD_COMMIT", ph.HeadCommit, + "$BASE_COMMIT", ph.BaseCommit, + "$COMMIT_RANGE", ph.CommitRange, + "$GIT_DIFF_HINT", ph.GitDiffHint, + "$REPORT_PATH", ph.ReportPath, + "$LABELS_PATH", ph.LabelsPath, + "$REPORT_TEMPLATE_PATH", ph.ReportTemplatePath, + ) + return replacer.Replace(string(templateBytes)), nil +} + +// gitDiffHint conveys how the agent should inspect the repository state. +func gitDiffHint(mode ReviewMode, baseSHA, headSHA string) string { + if mode == ReviewModeDifferential { + return fmt.Sprintf("Run `git diff %s...%s` (and related commands) inside %s to inspect the change set.", baseSHA, headSHA, defaultRepositoryPath) + } + return "Audit the entire working tree at the head commit." +} + +// modeLabel converts a review mode to a user friendly label. +func modeLabel(mode ReviewMode) string { + switch mode { + case ReviewModeDifferential: + return "Differential" + case ReviewModeFull: + return "Full" + default: + return "Unknown" + } +} + +// modeSummary explains the responsibilities associated with a review mode. +func modeSummary(mode ReviewMode) string { + switch mode { + case ReviewModeDifferential: + return "You are reviewing the changes introduced in a Git repository between the specified base and head commits. Prioritize spotting deliberately malicious additions alongside accidental vulnerabilities." + case ReviewModeFull: + return "You are auditing a Git repository snapshot at the specified head commit. Assume attackers may have hidden malicious logic and hunt for both intentional and accidental risks." + default: + return "The review mode is unknown." + } +} + +// fileExists returns true when a non-zero length file exists at path. +func fileExists(path string) bool { + info, err := os.Stat(path) + if err != nil { + return false + } + if info.Mode()&os.ModeType != 0 { + return false + } + return info.Size() > 0 +} + +// logInfo prints informational messages prefixed for clarity. +func logInfo(msg string) { + fmt.Printf("[security-reviewer] %s\n", msg) +} + +// logWarn prints warning messages prefixed for clarity. +func logWarn(msg string) { + fmt.Printf("[security-reviewer] WARNING: %s\n", msg) +} + +// logError prints error messages prefixed for clarity. +func logError(err error) { + var pathErr *fs.PathError + if errors.As(err, &pathErr) { + fmt.Fprintf(os.Stderr, "[security-reviewer] ERROR: %s (%s)\n", pathErr.Path, pathErr.Err) + return + } + fmt.Fprintf(os.Stderr, "[security-reviewer] ERROR: %s\n", err) +} + +// ensureDirectory verifies that the supplied path exists and is a directory. +func ensureDirectory(path string) error { + info, err := os.Stat(path) + if err != nil { + return fmt.Errorf("stat directory %s: %w", path, err) + } + if !info.IsDir() { + return fmt.Errorf("%s is not a directory", path) + } + return nil +} diff --git a/cmd/ci/collect_full_audit.go b/cmd/ci/collect_full_audit.go new file mode 100644 index 00000000..c8dbb804 --- /dev/null +++ b/cmd/ci/collect_full_audit.go @@ -0,0 +1,111 @@ +/* +Copyright © 2025 Docker, Inc. + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in +all copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +THE SOFTWARE. +*/ + +package main + +import ( + "errors" + "flag" + "io/fs" + "path/filepath" + "strings" +) + +// runCollectFullAudit enumerates local servers (optionally filtered) and writes +// their metadata to a JSON file for manual auditing. It expects --workspace, +// --servers, and --output-json flags. +func runCollectFullAudit(args []string) error { + flags := flag.NewFlagSet("collect-full-audit", flag.ContinueOnError) + workspace := flags.String("workspace", ".", "path to repository workspace") + filter := flags.String("servers", "", "optional comma-separated server filter") + outputJSON := flags.String("output-json", "", "path to write JSON context") + if err := flags.Parse(args); err != nil { + return err + } + + if *outputJSON == "" { + return errors.New("output-json is required") + } + + targets, err := collectAuditTargets(*workspace, *filter) + if err != nil { + return err + } + + if len(targets) == 0 { + removeIfPresent(*outputJSON) + return nil + } + + return writeJSONFile(*outputJSON, targets) +} + +// collectAuditTargets returns audit targets for all local servers or a filtered +// subset based on the supplied comma-separated list. +func collectAuditTargets(workspace, filter string) ([]auditTarget, error) { + filterSet := make(map[string]struct{}) + for _, name := range splitList(filter) { + filterSet[name] = struct{}{} + } + + var targets []auditTarget + err := filepath.WalkDir(filepath.Join(workspace, "servers"), func(path string, entry fs.DirEntry, walkErr error) error { + if walkErr != nil { + return walkErr + } + if entry.IsDir() || !strings.HasSuffix(path, "server.yaml") { + return nil + } + + relative := strings.TrimPrefix(path, workspace+string(filepath.Separator)) + doc, err := loadServerYAMLFromWorkspace(workspace, relative) + if err != nil || !isLocalServer(doc) { + return nil + } + + server := filepath.Base(filepath.Dir(path)) + if len(filterSet) > 0 { + if _, ok := filterSet[strings.ToLower(server)]; !ok { + return nil + } + } + + project := strings.TrimSpace(doc.Source.Project) + commit := strings.TrimSpace(doc.Source.Commit) + if project == "" || commit == "" { + return nil + } + + targets = append(targets, auditTarget{ + Server: server, + Project: project, + Commit: commit, + Directory: strings.TrimSpace(doc.Source.Directory), + }) + return nil + }) + if err != nil { + return nil, err + } + + return targets, nil +} diff --git a/cmd/ci/collect_new_servers.go b/cmd/ci/collect_new_servers.go new file mode 100644 index 00000000..517aebef --- /dev/null +++ b/cmd/ci/collect_new_servers.go @@ -0,0 +1,137 @@ +/* +Copyright © 2025 Docker, Inc. + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in +all copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +THE SOFTWARE. +*/ + +package main + +import ( + "errors" + "flag" + "fmt" + "os" + "path/filepath" + "strings" +) + +// runCollectNewServers identifies newly added local servers between two git +// revisions. It accepts --base, --head, --workspace, --output-json, and +// --summary-md flags, writing machine-readable targets and a Markdown summary +// for reviewers. +func runCollectNewServers(args []string) error { + flags := flag.NewFlagSet("collect-new-servers", flag.ContinueOnError) + base := flags.String("base", "", "base git commit SHA") + head := flags.String("head", "", "head git commit SHA") + workspace := flags.String("workspace", ".", "path to repository workspace") + outputJSON := flags.String("output-json", "", "path to write JSON context") + summaryMD := flags.String("summary-md", "", "path to write Markdown summary") + if err := flags.Parse(args); err != nil { + return err + } + + if *base == "" || *head == "" || *outputJSON == "" || *summaryMD == "" { + return errors.New("base, head, output-json, and summary-md are required") + } + + targets, err := collectNewServerTargets(*workspace, *base, *head) + if err != nil { + return err + } + + if len(targets) == 0 { + removeIfPresent(*outputJSON) + removeIfPresent(*summaryMD) + return nil + } + + if err := writeJSONFile(*outputJSON, targets); err != nil { + return err + } + + summary := buildNewServerSummary(targets) + return os.WriteFile(*summaryMD, []byte(summary), 0o644) +} + +// collectNewServerTargets returns metadata for local servers that were added +// between the supplied git revisions. +func collectNewServerTargets(workspace, base, head string) ([]newServerTarget, error) { + lines, err := gitDiff(workspace, base, head, "--name-status") + if err != nil { + return nil, err + } + + var targets []newServerTarget + for _, line := range lines { + if !strings.HasPrefix(line, "A\t") { + continue + } + path := strings.TrimPrefix(line, "A\t") + if !strings.HasPrefix(path, "servers/") || !strings.HasSuffix(path, "server.yaml") { + continue + } + + doc, err := loadServerYAMLFromWorkspace(workspace, path) + if err != nil { + continue + } + + if !isLocalServer(doc) { + continue + } + + project := strings.TrimSpace(doc.Source.Project) + commit := strings.TrimSpace(doc.Source.Commit) + if project == "" || commit == "" { + continue + } + + targets = append(targets, newServerTarget{ + Server: filepath.Base(filepath.Dir(path)), + File: path, + Image: strings.TrimSpace(doc.Image), + Project: project, + Commit: commit, + Directory: strings.TrimSpace(doc.Source.Directory), + }) + } + + return targets, nil +} + +// buildNewServerSummary renders Markdown describing newly added servers for +// review prompts and human consumption. +func buildNewServerSummary(targets []newServerTarget) string { + builder := strings.Builder{} + builder.WriteString("## New Local Servers\n\n") + + for _, target := range targets { + builder.WriteString(fmt.Sprintf("### %s\n", target.Server)) + builder.WriteString(fmt.Sprintf("- Repository: %s\n", target.Project)) + builder.WriteString(fmt.Sprintf("- Commit: `%s`\n", target.Commit)) + if target.Directory != "" { + builder.WriteString(fmt.Sprintf("- Directory: %s\n", target.Directory)) + } else { + builder.WriteString("- Directory: (repository root)\n") + } + builder.WriteString(fmt.Sprintf("- Checkout path: /tmp/security-review/new/%s/repo\n\n", target.Server)) + } + + return builder.String() +} diff --git a/cmd/ci/collect_updated_pins.go b/cmd/ci/collect_updated_pins.go new file mode 100644 index 00000000..0b85c27c --- /dev/null +++ b/cmd/ci/collect_updated_pins.go @@ -0,0 +1,141 @@ +/* +Copyright © 2025 Docker, Inc. + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in +all copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +THE SOFTWARE. +*/ + +package main + +import ( + "errors" + "flag" + "fmt" + "os" + "path/filepath" + "strings" +) + +// runCollectUpdatedPins gathers metadata for servers that updated their commit +// pins between two git revisions. It expects --base, --head, --workspace, +// --output-json, and --summary-md arguments. The identified targets are written +// to the JSON file while a Markdown summary is produced for humans. +func runCollectUpdatedPins(args []string) error { + flags := flag.NewFlagSet("collect-updated-pins", flag.ContinueOnError) + base := flags.String("base", "", "base git commit SHA") + head := flags.String("head", "", "head git commit SHA") + workspace := flags.String("workspace", ".", "path to repository workspace") + outputJSON := flags.String("output-json", "", "path to write JSON context") + summaryMD := flags.String("summary-md", "", "path to write Markdown summary") + if err := flags.Parse(args); err != nil { + return err + } + + if *base == "" || *head == "" || *outputJSON == "" || *summaryMD == "" { + return errors.New("base, head, output-json, and summary-md are required") + } + + targets, err := collectUpdatedPinTargets(*workspace, *base, *head) + if err != nil { + return err + } + + if len(targets) == 0 { + removeIfPresent(*outputJSON) + removeIfPresent(*summaryMD) + return nil + } + + if err := writeJSONFile(*outputJSON, targets); err != nil { + return err + } + + summary := buildPinSummary(targets) + return os.WriteFile(*summaryMD, []byte(summary), 0o644) +} + +// collectUpdatedPinTargets identifies local servers whose pinned commits differ +// between the supplied git revisions and returns their metadata for further +// processing. +func collectUpdatedPinTargets(workspace, base, head string) ([]pinTarget, error) { + paths, err := gitDiff(workspace, base, head, "--name-only") + if err != nil { + return nil, err + } + + var targets []pinTarget + for _, relative := range paths { + if !strings.HasPrefix(relative, "servers/") || !strings.HasSuffix(relative, "server.yaml") { + continue + } + + baseDoc, err := loadServerYAMLAt(workspace, base, relative) + if err != nil { + continue + } + headDoc, err := loadServerYAMLFromWorkspace(workspace, relative) + if err != nil { + continue + } + + if !isLocalServer(headDoc) || !isLocalServer(baseDoc) { + continue + } + + oldCommit := strings.TrimSpace(baseDoc.Source.Commit) + newCommit := strings.TrimSpace(headDoc.Source.Commit) + project := strings.TrimSpace(headDoc.Source.Project) + if oldCommit == "" || newCommit == "" || oldCommit == newCommit || project == "" { + continue + } + + targets = append(targets, pinTarget{ + Server: filepath.Base(filepath.Dir(relative)), + File: relative, + Image: strings.TrimSpace(headDoc.Image), + Project: project, + Directory: strings.TrimSpace(headDoc.Source.Directory), + OldCommit: oldCommit, + NewCommit: newCommit, + }) + } + + return targets, nil +} + +// buildPinSummary renders a Markdown section describing updated pin targets so +// that review tooling and humans can understand what changed. +func buildPinSummary(targets []pinTarget) string { + builder := strings.Builder{} + builder.WriteString("## Updated Commit Pins\n\n") + + for _, target := range targets { + builder.WriteString(fmt.Sprintf("### %s\n", target.Server)) + builder.WriteString(fmt.Sprintf("- Repository: %s\n", target.Project)) + if target.Directory != "" { + builder.WriteString(fmt.Sprintf("- Directory: %s\n", target.Directory)) + } else { + builder.WriteString("- Directory: (repository root)\n") + } + builder.WriteString(fmt.Sprintf("- Previous commit: `%s`\n", target.OldCommit)) + builder.WriteString(fmt.Sprintf("- New commit: `%s`\n", target.NewCommit)) + builder.WriteString(fmt.Sprintf("- Diff path: /tmp/security-review/pins/%s/diff.patch\n\n", target.Server)) + } + + return builder.String() +} diff --git a/cmd/ci/helpers.go b/cmd/ci/helpers.go new file mode 100644 index 00000000..7b8f72a7 --- /dev/null +++ b/cmd/ci/helpers.go @@ -0,0 +1,146 @@ +/* +Copyright © 2025 Docker, Inc. + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in +all copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +THE SOFTWARE. +*/ + +package main + +import ( + "encoding/json" + "fmt" + "os" + "os/exec" + "path/filepath" + "strings" + + "gopkg.in/yaml.v3" + + "github.com/docker/mcp-registry/pkg/servers" +) + +// writeJSONFile stores the provided value as indented JSON at the given path. +func writeJSONFile(path string, value any) error { + payload, err := json.MarshalIndent(value, "", " ") + if err != nil { + return err + } + return os.WriteFile(path, payload, 0o644) +} + +// readJSONFile populates value with JSON data read from the provided path. +func readJSONFile(path string, value any) error { + content, err := os.ReadFile(path) + if err != nil { + return err + } + return json.Unmarshal(content, value) +} + +// removeIfPresent deletes the file at the path when it exists. +func removeIfPresent(path string) { + if path == "" { + return + } + if _, err := os.Stat(path); err == nil { + _ = os.Remove(path) + } +} + +// loadServerYAMLFromWorkspace loads a server YAML file located in the workspace. +func loadServerYAMLFromWorkspace(workspace, relative string) (servers.Server, error) { + fullPath := filepath.Join(workspace, relative) + content, err := os.ReadFile(fullPath) + if err != nil { + return servers.Server{}, err + } + return decodeServerDocument(content) +} + +// loadServerYAMLAt loads a server YAML file from the git history at the commit. +func loadServerYAMLAt(workspace, commit, relative string) (servers.Server, error) { + out, err := runGitCommand(workspace, "show", fmt.Sprintf("%s:%s", commit, relative)) + if err != nil { + return servers.Server{}, err + } + return decodeServerDocument([]byte(out)) +} + +// decodeServerDocument converts raw YAML bytes into a servers.Server. +func decodeServerDocument(raw []byte) (servers.Server, error) { + var doc servers.Server + if err := yaml.Unmarshal(raw, &doc); err != nil { + return servers.Server{}, err + } + return doc, nil +} + +// isLocalServer returns true when the definition corresponds to a local server image. +func isLocalServer(doc servers.Server) bool { + if !strings.EqualFold(doc.Type, "server") { + return false + } + return strings.HasPrefix(strings.TrimSpace(doc.Image), "mcp/") +} + +// gitDiff runs git diff for server YAML files and returns the resulting paths. +func gitDiff(workspace, base, head, mode string) ([]string, error) { + args := []string{"diff", mode, base, head, "--", "servers/*/server.yaml"} + out, err := runGitCommand(workspace, args...) + if err != nil { + return nil, err + } + + var lines []string + for _, line := range strings.Split(strings.TrimSpace(out), "\n") { + line = strings.TrimSpace(line) + if line != "" { + lines = append(lines, line) + } + } + return lines, nil +} + +// runGitCommand executes git with the given arguments inside the directory. +func runGitCommand(dir string, args ...string) (string, error) { + cmd := exec.Command("git", args...) + cmd.Dir = dir + output, err := cmd.CombinedOutput() + if err != nil { + return "", fmt.Errorf("git %s: %w\n%s", strings.Join(args, " "), err, string(output)) + } + return string(output), nil +} + +// splitList normalizes a delimited string into lowercase server names. +func splitList(raw string) []string { + if raw == "" { + return nil + } + var values []string + for _, segment := range strings.FieldsFunc(raw, func(r rune) bool { + return r == ',' || r == '\n' || r == ' ' || r == '\t' + }) { + value := strings.TrimSpace(segment) + if value != "" { + values = append(values, strings.ToLower(value)) + } + } + return values +} diff --git a/cmd/ci/main.go b/cmd/ci/main.go new file mode 100644 index 00000000..c2c66fb9 --- /dev/null +++ b/cmd/ci/main.go @@ -0,0 +1,59 @@ +/* +Copyright © 2025 Docker, Inc. + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in +all copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +THE SOFTWARE. +*/ + +package main + +import ( + "fmt" + "os" +) + +// main dispatches the CLI to a specific sub-command implementation. +func main() { + if len(os.Args) < 2 { + fmt.Fprintln(os.Stderr, "usage: ci [options]") + os.Exit(2) + } + + cmd := os.Args[1] + args := os.Args[2:] + + var err error + switch cmd { + case "collect-updated-pins": + err = runCollectUpdatedPins(args) + case "collect-new-servers": + err = runCollectNewServers(args) + case "collect-full-audit": + err = runCollectFullAudit(args) + case "update-pins": + err = runUpdatePins(args) + default: + fmt.Fprintf(os.Stderr, "unknown command: %s\n", cmd) + os.Exit(2) + } + + if err != nil { + fmt.Fprintln(os.Stderr, err) + os.Exit(1) + } +} diff --git a/cmd/ci/types.go b/cmd/ci/types.go new file mode 100644 index 00000000..98d95572 --- /dev/null +++ b/cmd/ci/types.go @@ -0,0 +1,69 @@ +/* +Copyright © 2025 Docker, Inc. + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in +all copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +THE SOFTWARE. +*/ + +package main + +// pinTarget describes a server that updated its commit pin within a pull request. +type pinTarget struct { + // Server is the registry entry name (directory) that was updated. + Server string `json:"server"` + // File is the relative YAML path that changed for the server. + File string `json:"file"` + // Image is the Docker image identifier associated with the server. + Image string `json:"image"` + // Project is the upstream repository URL for the server source. + Project string `json:"project"` + // Directory points to the subdirectory inside the upstream repository, when set. + Directory string `json:"directory,omitempty"` + // OldCommit contains the previously pinned commit SHA. + OldCommit string `json:"old_commit"` + // NewCommit contains the newly pinned commit SHA. + NewCommit string `json:"new_commit"` +} + +// newServerTarget captures metadata for a newly added local server. +type newServerTarget struct { + // Server is the registry entry name for the newly added server. + Server string `json:"server"` + // File is the YAML file that defines the server in the registry. + File string `json:"file"` + // Image is the Docker image identifier associated with the new server. + Image string `json:"image"` + // Project is the upstream repository URL that hosts the server code. + Project string `json:"project"` + // Commit is the pinned commit SHA for the newly added server. + Commit string `json:"commit"` + // Directory specifies a subdirectory inside the upstream repository, when present. + Directory string `json:"directory,omitempty"` +} + +// auditTarget represents a server selected for a manual full audit. +type auditTarget struct { + // Server is the registry entry name included in the manual audit. + Server string `json:"server"` + // Project is the upstream repository URL for the audited server. + Project string `json:"project"` + // Commit is the pinned commit SHA to audit. + Commit string `json:"commit"` + // Directory is the subdirectory within the upstream repo to inspect, when applicable. + Directory string `json:"directory,omitempty"` +} diff --git a/cmd/ci/update_pins.go b/cmd/ci/update_pins.go new file mode 100644 index 00000000..0f2a368d --- /dev/null +++ b/cmd/ci/update_pins.go @@ -0,0 +1,193 @@ +/* +Copyright © 2025 Docker, Inc. + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in +all copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +THE SOFTWARE. +*/ + +package main + +import ( + "context" + "errors" + "fmt" + "os" + "path/filepath" + "regexp" + "sort" + "strings" + + "github.com/docker/mcp-registry/pkg/github" + "github.com/docker/mcp-registry/pkg/servers" +) + +// runUpdatePins refreshes pinned commits for local servers by resolving the +// latest upstream revision on the tracked branch and updating server YAML +// definitions in place. It does not take any CLI flags and emits a summary of +// modified servers on stdout; errors are reported per server so that a single +// failure does not abort the entire sweep. +func runUpdatePins(args []string) error { + if len(args) != 0 { + return errors.New("update-pins does not accept additional arguments") + } + + ctx := context.Background() + + entries, err := os.ReadDir("servers") + if err != nil { + return fmt.Errorf("reading servers directory: %w", err) + } + + var updated []string + for _, entry := range entries { + if !entry.IsDir() { + continue + } + + serverPath := filepath.Join("servers", entry.Name(), "server.yaml") + + // Parse the server definition so that we can evaluate eligibility and + // discover the backing GitHub repository and branch information. + server, err := servers.Read(serverPath) + if err != nil { + fmt.Fprintf(os.Stderr, "reading %s: %v\n", serverPath, err) + continue + } + + if server.Type != "server" { + continue + } + if !strings.HasPrefix(server.Image, "mcp/") { + continue + } + if server.Source.Project == "" { + continue + } + + // Only repositories hosted on GitHub can be advanced by this command, + // because the helper client relies on the GitHub API for commit lookup. + if !strings.Contains(server.Source.Project, "github.com/") { + fmt.Printf("Skipping %s: project is not hosted on GitHub.\n", server.Name) + continue + } + + existing := strings.ToLower(server.Source.Commit) + if existing == "" { + fmt.Printf("Skipping %s: no pinned commit present.\n", server.Name) + continue + } + + client := github.NewFromServer(server) + // Resolve the latest commit on the configured branch so we can refresh + // the pin if it has advanced since the last sweep. + latest, err := client.GetCommitSHA1(ctx, server.Source.Project, server.GetBranch()) + if err != nil { + fmt.Fprintf(os.Stderr, "fetching commit for %s: %v\n", server.Name, err) + continue + } + latest = strings.ToLower(latest) + + changed, err := writePinnedCommit(serverPath, latest) + if err != nil { + fmt.Fprintf(os.Stderr, "updating %s: %v\n", server.Name, err) + continue + } + + if existing != latest { + fmt.Printf("Updated %s: %s -> %s\n", server.Name, existing, latest) + } else if changed { + fmt.Printf("Reformatted pinned commit for %s at %s\n", server.Name, latest) + } + + if changed { + updated = append(updated, server.Name) + } + } + + if len(updated) == 0 { + fmt.Println("No commit updates required.") + return nil + } + + sort.Strings(updated) + fmt.Println("Servers with updated pins:", strings.Join(updated, ", ")) + return nil +} + +// writePinnedCommit replaces the commit field inside the source block with the +// provided SHA while preserving formatting. A boolean indicates whether the +// file changed. +func writePinnedCommit(path string, updated string) (bool, error) { + content, err := os.ReadFile(path) + if err != nil { + return false, err + } + + lines := strings.Split(string(content), "\n") + sourceIndex := -1 + for i, line := range lines { + if strings.HasPrefix(line, "source:") { + sourceIndex = i + break + } + } + if sourceIndex == -1 { + return false, fmt.Errorf("no source block found in %s", path) + } + + // Scan the nested source block until we locate the commit attribute, + // capturing its indentation so that formatting survives the rewrite. + commitIndex := -1 + indent := "" + commitPattern := regexp.MustCompile(`^([ \t]+)commit:\s*[a-fA-F0-9]{40}\s*$`) + for i := sourceIndex + 1; i < len(lines); i++ { + line := lines[i] + if !strings.HasPrefix(line, " ") { + break + } + + if match := commitPattern.FindStringSubmatch(line); match != nil { + commitIndex = i + indent = match[1] + break + } + } + + if commitIndex < 0 { + return false, fmt.Errorf("no commit line found in %s", path) + } + + // Replace only the commit value so that other keys maintain their + // original ordering and indentation. + newLine := indent + "commit: " + updated + lines[commitIndex] = newLine + + output := strings.Join(lines, "\n") + if !strings.HasSuffix(output, "\n") { + output += "\n" + } + + if output == string(content) { + return false, nil + } + + if err := os.WriteFile(path, []byte(output), 0o644); err != nil { + return false, err + } + return true, nil +} diff --git a/cmd/security-reviewer/main.go b/cmd/security-reviewer/main.go new file mode 100644 index 00000000..bc992cc5 --- /dev/null +++ b/cmd/security-reviewer/main.go @@ -0,0 +1,451 @@ +package main + +import ( + "context" + "errors" + "fmt" + "io" + "os" + "os/exec" + "os/signal" + "path/filepath" + "regexp" + "strings" + "syscall" + "time" + + "github.com/spf13/cobra" +) + +const ( + // composeFileName is the compose manifest executed for each review run. + composeFileName = "compose.yml" + // reportFileName is the name of the individual report emitted by the agent. + reportFileName = "report.md" + // labelsFileName is the name of the label output emitted by the agent. + labelsFileName = "labels.txt" + // repositoryDirName is the working directory used to stage repository clones. + repositoryDirName = "repository" + // dockerExecutable identifies the docker CLI binary invoked by the tool. + dockerExecutable = "docker" + // gitExecutable identifies the git CLI binary used to manage repositories. + gitExecutable = "git" + // projectPrefix is applied to compose project names to make them unique yet readable. + projectPrefix = "security-reviewer" + // agentService is the compose service name running the security reviewer container. + agentService = "reviewer" + // composeRelativePath is the path to the compose project relative to the repository root. + composeRelativePath = "agents/security-reviewer" + // defaultTimeoutSeconds bounds agent execution time when not explicitly configured. + defaultTimeoutSeconds = 3600 + + // envAnthropicAPIKey is the environment variable supplying Claude credentials. + envAnthropicAPIKey = "ANTHROPIC_API_KEY" + // envOpenAIAPIKey is the environment variable supplying Codex credentials. + envOpenAIAPIKey = "OPENAI_API_KEY" + + // agentNameClaude identifies the Claude-based reviewer. + agentNameClaude = "claude" + // agentNameCodex identifies the Codex-based reviewer. + agentNameCodex = "codex" +) + +// options stores parsed CLI arguments. +type options struct { + // Agent selects the underlying reviewer agent implementation. + Agent string + // Repository is the Git repository URL or filesystem path. + Repository string + // HeadSHA is the commit under audit. + HeadSHA string + // BaseSHA is the comparison commit for differential reviews. + BaseSHA string + // TargetLabel is an optional human friendly descriptor. + TargetLabel string + // OutputPath is the destination for the final report. + OutputPath string + // LabelsOutput is the destination for the label list produced by the reviewer. + LabelsOutput string + // Model optionally overrides the reviewer model selection. + Model string + // ExtraArgs optionally appends raw arguments to the agent CLI. + ExtraArgs string + // KeepWorkdir preserves the temporary workspace when true. + KeepWorkdir bool + // TimeoutSeconds bounds the reviewer runtime; zero uses the default. + TimeoutSeconds int +} + +var cliOpts = options{ + Agent: agentNameClaude, + OutputPath: "security-review.md", + TimeoutSeconds: defaultTimeoutSeconds, +} + +var rootCmd = &cobra.Command{ + Use: "security-reviewer", + Short: "Run the security reviewer compose workflow", + RunE: func(cmd *cobra.Command, args []string) error { + agent := strings.ToLower(strings.TrimSpace(cliOpts.Agent)) + if agent == "" { + agent = agentNameClaude + } + if agent != agentNameClaude && agent != agentNameCodex { + return fmt.Errorf("invalid agent %q (supported: %s, %s)", cliOpts.Agent, agentNameClaude, agentNameCodex) + } + + labelsOutput := strings.TrimSpace(cliOpts.LabelsOutput) + if labelsOutput == "" { + labelsOutput = deriveDefaultLabelsPath(cliOpts.OutputPath) + } + timeoutSecs := cliOpts.TimeoutSeconds + if timeoutSecs <= 0 { + timeoutSecs = defaultTimeoutSeconds + } + + opts := options{ + Agent: agent, + Repository: strings.TrimSpace(cliOpts.Repository), + HeadSHA: strings.TrimSpace(cliOpts.HeadSHA), + BaseSHA: strings.TrimSpace(cliOpts.BaseSHA), + TargetLabel: strings.TrimSpace(cliOpts.TargetLabel), + OutputPath: strings.TrimSpace(cliOpts.OutputPath), + LabelsOutput: labelsOutput, + Model: strings.TrimSpace(cliOpts.Model), + ExtraArgs: strings.TrimSpace(cliOpts.ExtraArgs), + KeepWorkdir: cliOpts.KeepWorkdir, + TimeoutSeconds: timeoutSecs, + } + + if opts.Repository == "" { + return errors.New("--repo is required") + } + if opts.HeadSHA == "" { + return errors.New("--head is required") + } + ctx := cmd.Context() + return run(ctx, opts) + }, +} + +func init() { + rootCmd.Flags().StringVar(&cliOpts.Agent, "agent", cliOpts.Agent, "Reviewer agent to use (claude or codex).") + rootCmd.Flags().StringVar(&cliOpts.Repository, "repo", cliOpts.Repository, "Git repository URL or local path to review.") + rootCmd.Flags().StringVar(&cliOpts.HeadSHA, "head", cliOpts.HeadSHA, "Head commit SHA to review.") + rootCmd.Flags().StringVar(&cliOpts.BaseSHA, "base", cliOpts.BaseSHA, "Base commit SHA for differential reviews.") + rootCmd.Flags().StringVar(&cliOpts.TargetLabel, "target-label", cliOpts.TargetLabel, "Human readable identifier for the target.") + rootCmd.Flags().StringVar(&cliOpts.OutputPath, "output", cliOpts.OutputPath, "Destination for the rendered report.") + rootCmd.Flags().StringVar(&cliOpts.LabelsOutput, "labels-output", cliOpts.LabelsOutput, "Destination for the labels file (defaults alongside the report).") + rootCmd.Flags().IntVar(&cliOpts.TimeoutSeconds, "timeout", cliOpts.TimeoutSeconds, "Maximum runtime for the review in seconds (defaults to 3600 seconds).") + rootCmd.Flags().StringVar(&cliOpts.Model, "model", cliOpts.Model, "Override the reviewer model for the selected agent.") + rootCmd.Flags().StringVar(&cliOpts.ExtraArgs, "extra-args", cliOpts.ExtraArgs, "Additional arguments passed to the reviewer agent.") + rootCmd.Flags().BoolVar(&cliOpts.KeepWorkdir, "keep-workdir", cliOpts.KeepWorkdir, "Keep the temporary workspace after completion.") + + _ = rootCmd.MarkFlagRequired("repo") + _ = rootCmd.MarkFlagRequired("head") +} + +// main is the entry point for the security reviewer CLI. +func main() { + ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM) + defer stop() + rootCmd.SilenceUsage = true + if err := rootCmd.ExecuteContext(ctx); err != nil { + exitWithError(err) + } +} + +// run coordinates workspace preparation, compose execution, and cleanup. +func run(ctx context.Context, opts options) error { + if opts.BaseSHA != "" { + fmt.Printf( + "Starting differential security review (agent=%s head=%s base=%s target=%s)\n", + opts.Agent, + opts.HeadSHA, + opts.BaseSHA, + opts.TargetLabel, + ) + } else { + fmt.Printf( + "Starting full security review (agent=%s head=%s target=%s)\n", + opts.Agent, + opts.HeadSHA, + opts.TargetLabel, + ) + } + + // Ensure the credential proxy has the API keys it needs before staging work. + switch opts.Agent { + case "claude": + if _, ok := os.LookupEnv(envAnthropicAPIKey); !ok { + return errors.New("ANTHROPIC_API_KEY environment variable is required for the Claude agent") + } + case "codex": + if _, ok := os.LookupEnv(envOpenAIAPIKey); !ok { + return errors.New("OPENAI_API_KEY environment variable is required for the Codex agent") + } + } + + // Prepare a temporary workspace to stage inputs and outputs. + workdir, err := os.MkdirTemp("", fmt.Sprintf("security-reviewer-%s-", opts.Agent)) + if err != nil { + return fmt.Errorf("create temporary directory: %w", err) + } + + if !opts.KeepWorkdir { + defer os.RemoveAll(workdir) + } else { + fmt.Printf("Temporary workspace preserved at %s\n", workdir) + } + + // Materialize the repository commits required for the review. + repositoryDir := filepath.Join(workdir, repositoryDirName) + if err = prepareRepository(ctx, opts, repositoryDir); err != nil { + return err + } + + outputDir := filepath.Join(workdir, "output") + if err = os.MkdirAll(outputDir, 0o755); err != nil { + return fmt.Errorf("create output directory: %w", err) + } + + // Launch the compose project and wait for the reviewer to finish. + if err = runCompose(ctx, opts, workdir, repositoryDir, outputDir); err != nil { + return err + } + + // Copy the generated artifacts back to the requested destinations. + reportPath := filepath.Join(outputDir, reportFileName) + labelsPath := filepath.Join(outputDir, labelsFileName) + if _, err = os.Stat(reportPath); err != nil { + return fmt.Errorf("review report not produced: %w", err) + } + if _, err = os.Stat(labelsPath); err != nil { + return fmt.Errorf("labels file not produced: %w", err) + } + + if err = copyFile(reportPath, opts.OutputPath); err != nil { + return err + } + if err = copyFile(labelsPath, opts.LabelsOutput); err != nil { + return err + } + + fmt.Printf("Security review report copied to %s\n", opts.OutputPath) + fmt.Printf("Security review labels copied to %s\n", opts.LabelsOutput) + return nil +} + +// deriveDefaultLabelsPath produces a labels output path near the report path. +func deriveDefaultLabelsPath(reportPath string) string { + reportPath = strings.TrimSpace(reportPath) + if reportPath == "" { + // Without an explicit report, fall back to a stable default name. + return "security-review-labels.txt" + } + // Place the labels file alongside the report for easier discovery. + dir := filepath.Dir(reportPath) + base := filepath.Base(reportPath) + idx := strings.LastIndex(base, ".") + if idx > 0 { + // Drop the extension so the generated labels file mirrors the report name. + base = base[:idx] + } + if strings.TrimSpace(base) == "" { + base = "security-review" + } + // Append a suffix to distinguish the labels artifact from the report. + return filepath.Join(dir, base+"-labels.txt") +} + +// sanitizeName converts arbitrary text into a slug. +func sanitizeName(text string) string { + lower := strings.ToLower(text) + pattern := regexp.MustCompile(`[^a-z0-9]+`) + cleaned := pattern.ReplaceAllString(lower, "-") + trimmed := strings.Trim(cleaned, "-") + if trimmed == "" { + return "target" + } + return trimmed +} + +// prepareRepository clones the repository and materializes commits for review. +func prepareRepository(ctx context.Context, opts options, repositoryDir string) error { + parentDir := filepath.Dir(repositoryDir) + if err := os.MkdirAll(parentDir, 0o755); err != nil { + return fmt.Errorf("create repository parent directory: %w", err) + } + if err := os.RemoveAll(repositoryDir); err != nil { + return fmt.Errorf("reset repository directory: %w", err) + } + + if err := runCommand(ctx, "", gitExecutable, "clone", opts.Repository, repositoryDir); err != nil { + return fmt.Errorf("clone repository: %w", err) + } + + if err := ensureCommit(ctx, repositoryDir, opts.HeadSHA); err != nil { + return err + } + if err := runCommand(ctx, repositoryDir, gitExecutable, "checkout", "--detach", opts.HeadSHA); err != nil { + return fmt.Errorf("checkout head commit: %w", err) + } + + if opts.BaseSHA != "" { + if err := ensureCommit(ctx, repositoryDir, opts.BaseSHA); err != nil { + return err + } + } + + return nil +} + +// ensureCommit verifies that a commit exists locally, fetching if needed. +func ensureCommit(ctx context.Context, repoDir, sha string) error { + if sha == "" { + return nil + } + if err := runCommand(ctx, repoDir, gitExecutable, "rev-parse", "--verify", sha); err == nil { + return nil + } + if err := runCommand(ctx, repoDir, gitExecutable, "fetch", "origin", sha); err != nil { + return fmt.Errorf("fetch commit %s: %w", sha, err) + } + if err := runCommand(ctx, repoDir, gitExecutable, "rev-parse", "--verify", sha); err != nil { + return fmt.Errorf("verify commit %s: %w", sha, err) + } + return nil +} + +// copyFile copies a file from src to dst, creating parent directories. +func copyFile(src, dst string) error { + in, err := os.Open(src) + if err != nil { + return fmt.Errorf("open file %s: %w", src, err) + } + defer in.Close() + if err = os.MkdirAll(filepath.Dir(dst), 0o755); err != nil { + return fmt.Errorf("create directory for %s: %w", dst, err) + } + out, err := os.OpenFile(dst, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0o644) + if err != nil { + return fmt.Errorf("open destination %s: %w", dst, err) + } + defer out.Close() + if _, err = io.Copy(out, in); err != nil { + return fmt.Errorf("copy %s to %s: %w", src, dst, err) + } + return nil +} + +// runCompose executes the docker compose workflow for the review. +func runCompose(ctx context.Context, opts options, workdir, repositoryDir, outputDir string) error { + // Compose assumes relative paths, so stage a copy inside the temp workspace. + composeDir := filepath.Join(workdir, composeRelativePath) + if err := copyDir(composeRelativePath, composeDir); err != nil { + return err + } + + env := buildComposeEnv(opts, repositoryDir, outputDir) + up := exec.CommandContext(ctx, dockerExecutable, "compose", "-f", composeFileName, "up", "--build", "--abort-on-container-exit", "--exit-code-from", agentService) + up.Dir = composeDir + up.Env = env + up.Stdout = os.Stdout + up.Stderr = os.Stderr + + down := exec.CommandContext(context.Background(), dockerExecutable, "compose", "-f", composeFileName, "down", "--volumes", "--remove-orphans") + down.Dir = composeDir + down.Env = env + + if err := up.Run(); err != nil { + _ = down.Run() + return fmt.Errorf("docker compose up: %w", err) + } + if err := down.Run(); err != nil { + return fmt.Errorf("docker compose down: %w", err) + } + return nil +} + +// buildComposeEnv prepares environment variables for docker compose. +func buildComposeEnv(opts options, repositoryDir, outputDir string) []string { + env := os.Environ() + // Generate a stable slug to keep compose project names readable. + slug := sanitizeName(opts.TargetLabel) + if slug == "target" { + repoBase := filepath.Base(strings.TrimSuffix(opts.Repository, ".git")) + slug = sanitizeName(repoBase) + } + if slug == "" { + slug = "target" + } + projectName := fmt.Sprintf("%s-%s-%d", projectPrefix, slug, time.Now().Unix()) + env = append(env, + fmt.Sprintf("COMPOSE_PROJECT_NAME=%s", projectName), + fmt.Sprintf("REVIEW_AGENT=%s", opts.Agent), + fmt.Sprintf("REVIEW_HEAD_SHA=%s", opts.HeadSHA), + fmt.Sprintf("REVIEW_BASE_SHA=%s", opts.BaseSHA), + fmt.Sprintf("REVIEW_TARGET_LABEL=%s", opts.TargetLabel), + fmt.Sprintf("REVIEW_REPOSITORY_PATH=%s", repositoryDir), + fmt.Sprintf("REVIEW_OUTPUT_PATH=%s", outputDir), + fmt.Sprintf("REVIEW_TIMEOUT_SECS=%d", opts.TimeoutSeconds), + ) + if opts.Model != "" { + // Route custom models to the right environment variable per agent. + switch strings.ToLower(opts.Agent) { + case agentNameClaude: + env = append(env, fmt.Sprintf("CLAUDE_REVIEW_MODEL=%s", opts.Model)) + case agentNameCodex: + env = append(env, fmt.Sprintf("CODEX_REVIEW_MODEL=%s", opts.Model)) + } + } + if opts.ExtraArgs != "" { + env = append(env, fmt.Sprintf("REVIEW_AGENT_EXTRA_ARGS=%s", opts.ExtraArgs)) + } + if key := strings.TrimSpace(os.Getenv(envAnthropicAPIKey)); key != "" { + env = append(env, fmt.Sprintf("%s=%s", envAnthropicAPIKey, key)) + } + if key := strings.TrimSpace(os.Getenv(envOpenAIAPIKey)); key != "" { + env = append(env, fmt.Sprintf("%s=%s", envOpenAIAPIKey, key)) + } + return env +} + +// copyDir performs a recursive directory copy. +func copyDir(src, dst string) error { + return filepath.Walk(src, func(path string, info os.FileInfo, walkErr error) error { + if walkErr != nil { + return walkErr + } + rel, err := filepath.Rel(src, path) + if err != nil { + return err + } + target := filepath.Join(dst, rel) + if info.IsDir() { + return os.MkdirAll(target, info.Mode()) + } + data, err := os.ReadFile(path) + if err != nil { + return err + } + return os.WriteFile(target, data, info.Mode()) + }) +} + +// runCommand executes a command within an optional directory. +func runCommand(ctx context.Context, dir, name string, args ...string) error { + cmd := exec.CommandContext(ctx, name, args...) + if dir != "" { + cmd.Dir = dir + } + cmd.Stdout = os.Stdout + cmd.Stderr = os.Stderr + return cmd.Run() +} + +// exitWithError prints an error and terminates the process. +func exitWithError(err error) { + fmt.Fprintf(os.Stderr, "error: %v\n", err) + os.Exit(1) +} diff --git a/go.mod b/go.mod index 32f974ad..ff589edb 100644 --- a/go.mod +++ b/go.mod @@ -7,6 +7,7 @@ require ( github.com/charmbracelet/lipgloss v1.1.0 github.com/google/go-github/v70 v70.0.0 github.com/mark3labs/mcp-go v0.25.0 + github.com/spf13/cobra v1.8.0 gopkg.in/yaml.v3 v3.0.1 ) @@ -25,6 +26,7 @@ require ( github.com/erikgeiser/coninput v0.0.0-20211004153227-1c3628e74d0f // indirect github.com/google/go-querystring v1.1.0 // indirect github.com/google/uuid v1.6.0 // indirect + github.com/inconshreveable/mousetrap v1.1.0 // indirect github.com/lucasb-eyer/go-colorful v1.2.0 // indirect github.com/mattn/go-isatty v0.0.20 // indirect github.com/mattn/go-localereader v0.0.1 // indirect @@ -35,6 +37,7 @@ require ( github.com/muesli/termenv v0.16.0 // indirect github.com/rivo/uniseg v0.4.7 // indirect github.com/spf13/cast v1.7.1 // indirect + github.com/spf13/pflag v1.0.5 // indirect github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e // indirect github.com/yosida95/uritemplate/v3 v3.0.2 // indirect golang.org/x/sync v0.12.0 // indirect diff --git a/go.sum b/go.sum index 493a8b02..24ad4f3a 100644 --- a/go.sum +++ b/go.sum @@ -36,6 +36,7 @@ github.com/charmbracelet/x/termios v0.1.1 h1:o3Q2bT8eqzGnGPOYheoYS8eEleT5ZVNYNy8 github.com/charmbracelet/x/termios v0.1.1/go.mod h1:rB7fnv1TgOPOyyKRJ9o+AsTU/vK5WHJ2ivHeut/Pcwo= github.com/charmbracelet/x/xpty v0.1.2 h1:Pqmu4TEJ8KeA9uSkISKMU3f+C1F6OGBn8ABuGlqCbtI= github.com/charmbracelet/x/xpty v0.1.2/go.mod h1:XK2Z0id5rtLWcpeNiMYBccNNBrP2IJnzHI0Lq13Xzq4= +github.com/cpuguy83/go-md2man/v2 v2.0.3/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o= github.com/creack/pty v1.1.24 h1:bJrF4RRfyJnbTJqzRLHzcGaZK1NeM5kTC9jGgovnR1s= github.com/creack/pty v1.1.24/go.mod h1:08sCNb52WyoAwi2QDyzUCTgcvVFhUzewun7wtTfvcwE= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= @@ -55,6 +56,8 @@ github.com/google/go-querystring v1.1.0 h1:AnCroh3fv4ZBgVIf1Iwtovgjaw/GiKJo8M8yD github.com/google/go-querystring v1.1.0/go.mod h1:Kcdr2DB4koayq7X8pmAG4sNG59So17icRSOU623lUBU= github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0= github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= +github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2s0bqwp9tc8= +github.com/inconshreveable/mousetrap v1.1.0/go.mod h1:vpF70FUmC8bwa3OWnCshd2FqLfsEA9PFc4w1p2J65bw= github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE= github.com/kr/pretty v0.3.1/go.mod h1:hoEshYVHaxMs3cyo3Yncou5ZscifuDolrwPKZanG3xk= github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= @@ -84,8 +87,13 @@ github.com/rivo/uniseg v0.4.7 h1:WUdvkW8uEhrYfLC4ZzdpI2ztxP1I582+49Oc5Mq64VQ= github.com/rivo/uniseg v0.4.7/go.mod h1:FN3SvrM+Zdj16jyLfmOkMNblXMcoc8DfTHruCPUcx88= github.com/rogpeppe/go-internal v1.9.0 h1:73kH8U+JUqXU8lRuOHeVHaa/SZPifC7BkcraZVejAe8= github.com/rogpeppe/go-internal v1.9.0/go.mod h1:WtVeX8xhTBvf0smdhujwtBcq4Qrzq/fJaraNFVN+nFs= +github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= github.com/spf13/cast v1.7.1 h1:cuNEagBQEHWN1FnbGEjCXL2szYEXqfJPbP2HNUaca9Y= github.com/spf13/cast v1.7.1/go.mod h1:ancEpBxwJDODSW/UG4rDrAqiKolqNNh2DX3mk86cAdo= +github.com/spf13/cobra v1.8.0 h1:7aJaZx1B85qltLMc546zn58BxxfZdR/W22ej9CFoEf0= +github.com/spf13/cobra v1.8.0/go.mod h1:WXLWApfZ71AjXPya3WOlMsY9yMs7YeiHhFVlvLyhcho= +github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA= +github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= github.com/stretchr/testify v1.9.0 h1:HtqpIVDClZ4nwg75+f6Lvsy/wHu+3BoSGCbBAcpTsTg= github.com/stretchr/testify v1.9.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY= github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e h1:JVG44RsyaB9T2KIHavMF/ppJZNG9ZpyihvCd0w101no=