diff --git a/.github/claude-security-review-prompt.md b/.github/claude-security-review-prompt.md new file mode 100644 index 000000000000..ee9794c590fd --- /dev/null +++ b/.github/claude-security-review-prompt.md @@ -0,0 +1,22 @@ +Run the /security-review skill against the diff between +base $BASE_SHA and head $HEAD_SHA (PR #$PR_NUMBER). + +Follow the three-phase methodology (identify → parallel +false-positive filter → keep only confidence >= 8). + +After producing the markdown report, ALSO write a +machine-readable summary to $GITHUB_WORKSPACE/security-findings.json +with this schema: + +{ + "findings_count": , + "high_count": , + "medium_count": , + "report_markdown": "" +} + +If no findings clear the bar, write: +{"findings_count": 0, "high_count": 0, "medium_count": 0, "report_markdown": ""} + +DO NOT post any PR comment yourself. Downstream steps handle +disclosure routing. diff --git a/.github/workflows/claude-security-review.yml b/.github/workflows/claude-security-review.yml new file mode 100644 index 000000000000..79f0e5f84d8c --- /dev/null +++ b/.github/workflows/claude-security-review.yml @@ -0,0 +1,310 @@ +name: Claude Security Review + +# Runs an AI-assisted security review on every PR (opened / synchronize) and on +# manual dispatch. Claude always reviews — it catches logic-level issues (auth +# bypasses, injection reachable only through application flow) that pattern-based +# scanners miss, so its run is not gated on any other tool. On findings: +# - Posts an INTENTIONALLY ABSTRACT comment on the (public) PR +# - Notifies the PR author privately on Slack with full details +# - Fails the check so the PR is blocked from merge +# (only enforced when this job is added to branch-protection required checks) +# +# Required configuration (Settings → Secrets and variables → Actions): +# secrets: +# SLACK_BOT_TOKEN - Slack bot token with chat:write scope, invited to the channel +# SLACK_USER_MAP - JSON: {"github-login": "Uxxxxxxxx", ...} +# variables: +# BEDROCK_ROLE_ARN - IAM role (OIDC-assumable) with bedrock:InvokeModel; Claude runs on AWS Bedrock +# BEDROCK_MODEL_ID - Bedrock model ID, e.g. anthropic.claude-opus-4-8 (shared with the other AI workflows) +# BEDROCK_AWS_REGION - AWS region for Bedrock (defaults to us-east-1) +# SLACK_SECURITY_CHANNEL - Slack channel ID (e.g. C0XXXXXXX) for private security comms +# +# To make this block merges: +# Settings → Branches → main → Branch protection rule → +# "Require status checks to pass" → add "Claude Security Review / security-review" +# +# Security note: values from the GitHub context / step outputs (PR title, number, +# author, SHAs) are treated as untrusted. They are passed to +# actions/github-script via `env:` and read with process.env — never interpolated +# directly into a `script:` body, which would allow code injection. + +on: + pull_request: + types: [opened, synchronize] + workflow_dispatch: + inputs: + pr_number: + description: 'PR number to review' + required: true + type: string + +permissions: + contents: read + pull-requests: write + issues: write + checks: write + id-token: write # OIDC — assume the Bedrock role (no long-lived AWS keys) + +concurrency: + group: claude-security-review-${{ github.event.pull_request.number || inputs.pr_number }} + cancel-in-progress: true + +jobs: + security-review: + name: security-review + runs-on: ubuntu-latest + timeout-minutes: 25 + # Draft handling lives here (single source of truth): draft pull_request events + # never start a runner. A manual workflow_dispatch is a deliberate action, so it + # runs even on drafts. + if: github.event_name == 'workflow_dispatch' || github.event.pull_request.draft == false + + steps: + - name: Resolve PR details + id: pr + uses: actions/github-script@v7 + env: + # Passed via env rather than interpolated into the script so a crafted + # dispatch input cannot inject JavaScript (flagged by Semgrep). + PR_NUMBER_INPUT: ${{ inputs.pr_number }} + with: + github-token: ${{ secrets.GITHUB_TOKEN }} + script: | + let pr; + if (context.eventName === 'workflow_dispatch') { + const prNumber = Number.parseInt(process.env.PR_NUMBER_INPUT || '', 10); + if (!Number.isInteger(prNumber) || prNumber <= 0) { + core.setFailed('Invalid PR number'); + return; + } + const { data } = await github.rest.pulls.get({ + owner: context.repo.owner, + repo: context.repo.repo, + pull_number: prNumber + }); + pr = data; + core.info(`Resolved PR #${pr.number} via workflow_dispatch`); + } else { + pr = context.payload.pull_request; + } + core.setOutput('number', String(pr.number)); + core.setOutput('base_sha', pr.base.sha); + core.setOutput('head_sha', pr.head.sha); + core.setOutput('title', pr.title); + core.setOutput('html_url', pr.html_url); + core.setOutput('author', pr.user.login); + core.setOutput('is_draft', pr.draft ? 'true' : 'false'); + + # The prompt template lives in this workflow's own commit, not in the PR being + # reviewed. Fetch it from github.workflow_sha (the commit that defines this run) + # so it resolves correctly on any PR, on workflow_dispatch, and after this merges + # to main — with no hardcoded branch name. Staged to /tmp before the PR-head + # checkout so it survives that checkout's clean step. + - name: Fetch security-review prompt (from this workflow's own ref) + uses: actions/checkout@v4 + with: + ref: ${{ github.workflow_sha }} + sparse-checkout: .github/claude-security-review-prompt.md + sparse-checkout-cone-mode: false + path: _securityreview + + - name: Stage prompt template + run: cp _securityreview/.github/claude-security-review-prompt.md /tmp/claude-security-review-prompt.md + + - name: Checkout PR head + uses: actions/checkout@v4 + with: + ref: ${{ steps.pr.outputs.head_sha }} + fetch-depth: 0 + # Claude runs with Bash over this (attacker-controllable) PR code. Don't + # leave a GITHUB_TOKEN in .git/config for a prompt-injection payload to reach; + # nothing downstream of this checkout performs an authenticated git operation. + persist-credentials: false + + - name: Prepare prompt (substitute runtime values) + env: + BASE_SHA: ${{ steps.pr.outputs.base_sha }} + HEAD_SHA: ${{ steps.pr.outputs.head_sha }} + PR_NUMBER: ${{ steps.pr.outputs.number }} + run: envsubst < /tmp/claude-security-review-prompt.md > /tmp/claude-prompt.md + + - name: Install Claude Code CLI + run: npm install -g @anthropic-ai/claude-code@latest --quiet + + - name: Configure AWS credentials (Bedrock) + uses: aws-actions/configure-aws-credentials@v4 + with: + role-to-assume: ${{ vars.BEDROCK_ROLE_ARN }} + aws-region: ${{ vars.BEDROCK_AWS_REGION || 'us-east-1' }} + + - name: Run Claude security review + id: review + env: + # Route Claude through AWS Bedrock (OIDC-assumed role above) rather than the + # Anthropic API, so model usage runs inside dotCMS's own AWS controls/billing. + # Matches the pattern in issue_autodoc / the other ai_claude-* workflows. + CLAUDE_CODE_USE_BEDROCK: '1' + AWS_REGION: ${{ vars.BEDROCK_AWS_REGION || 'us-east-1' }} + BEDROCK_MODEL_ID: ${{ vars.BEDROCK_MODEL_ID }} + run: | + # Run Claude headlessly. The prompt instructs Claude to write + # security-findings.json to the workspace. + claude \ + --dangerously-skip-permissions \ + --allowedTools "Bash,Read,Grep,Glob,Agent" \ + --model "$BEDROCK_MODEL_ID" \ + -p "$(cat /tmp/claude-prompt.md)" + + - name: Parse findings + id: parse + run: | + set -euo pipefail + # Fail closed: a completed review ALWAYS writes security-findings.json + # (with findings_count: 0 when clean — see the prompt template). A missing + # file means the review did not complete (Claude errored, timed out, hit an + # API/quota failure, or wrote nothing). Do NOT treat that as "clean" — a + # green check must mean "reviewed and nothing found." + if [[ ! -f security-findings.json ]]; then + echo "::error::Claude security review did not complete — security-findings.json was not produced. Failing the check (fail closed)." + exit 1 + fi + count=$(jq -r '.findings_count // 0' security-findings.json) + high=$(jq -r '.high_count // 0' security-findings.json) + medium=$(jq -r '.medium_count // 0' security-findings.json) + if [[ "$count" -gt 0 ]]; then + has_findings=true + else + has_findings=false + fi + { + echo "findings_count=$count" + echo "high_count=$high" + echo "medium_count=$medium" + echo "has_findings=$has_findings" + } >> "$GITHUB_OUTPUT" + + - name: Post abstract PR comment (findings) + if: steps.parse.outputs.has_findings == 'true' + uses: actions/github-script@v7 + env: + PR_NUMBER: ${{ steps.pr.outputs.number }} + FINDINGS_COUNT: ${{ steps.parse.outputs.findings_count }} + with: + github-token: ${{ secrets.GITHUB_TOKEN }} + script: | + const n = process.env.FINDINGS_COUNT || '0'; + const body = [ + '## Security Review', + '', + `Automated security review flagged **${n} issue(s)** that require attention before this PR can merge.`, + '', + 'For security reasons, details are not posted here. The PR author has been notified privately on Slack with the full report and remediation guidance.', + '', + 'If you did not receive a Slack notification, contact the security team.', + '', + '_This check will remain red until the findings are resolved and a new commit is pushed._' + ].join('\n'); + await github.rest.issues.createComment({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: Number.parseInt(process.env.PR_NUMBER || '', 10), + body + }); + + - name: Post clean PR comment (no findings) + if: steps.parse.outputs.has_findings == 'false' + uses: actions/github-script@v7 + env: + PR_NUMBER: ${{ steps.pr.outputs.number }} + with: + github-token: ${{ secrets.GITHUB_TOKEN }} + script: | + const body = [ + '## Security Review', + '', + 'No high-confidence security findings on the changes in this PR.' + ].join('\n'); + await github.rest.issues.createComment({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: Number.parseInt(process.env.PR_NUMBER || '', 10), + body + }); + + - name: Notify author privately on Slack + if: steps.parse.outputs.has_findings == 'true' + env: + SLACK_BOT_TOKEN: ${{ secrets.SLACK_BOT_TOKEN }} + SLACK_USER_MAP: ${{ secrets.SLACK_USER_MAP }} + SLACK_CHANNEL: ${{ vars.SLACK_SECURITY_CHANNEL }} + PR_AUTHOR: ${{ steps.pr.outputs.author }} + PR_URL: ${{ steps.pr.outputs.html_url }} + PR_TITLE: ${{ steps.pr.outputs.title }} + PR_NUMBER: ${{ steps.pr.outputs.number }} + FINDINGS_COUNT: ${{ steps.parse.outputs.findings_count }} + run: | + set -euo pipefail + python3 - <<'PY' + import json, os, sys, urllib.request + + with open("security-findings.json") as f: + data = json.load(f) + report = data.get("report_markdown", "") or "(no report content)" + + token = os.environ["SLACK_BOT_TOKEN"] + channel = os.environ["SLACK_CHANNEL"] + author = os.environ["PR_AUTHOR"] + pr_url = os.environ["PR_URL"] + pr_t = os.environ["PR_TITLE"] + pr_n = os.environ["PR_NUMBER"] + count = os.environ["FINDINGS_COUNT"] + + user_map = json.loads(os.environ.get("SLACK_USER_MAP") or "{}") + slack_uid = user_map.get(author) + mention = f"<@{slack_uid}>" if slack_uid else f"`{author}` _(no Slack mapping — add to SLACK_USER_MAP)_" + + # Slack section text blocks are limited to ~3000 chars; chunk the report. + MAX = 2800 + chunks = [report[i:i+MAX] for i in range(0, len(report), MAX)] or [""] + report_blocks = [ + {"type": "section", "text": {"type": "mrkdwn", "text": "```\n" + c + "\n```"}} + for c in chunks[:8] # cap at 8 blocks to stay under Slack's 50-block limit + ] + + payload = { + "channel": channel, + "text": f"Security findings on PR #{pr_n}", + "blocks": [ + {"type": "header", "text": {"type": "plain_text", "text": "Security Review — Action Required"}}, + {"type": "section", "text": {"type": "mrkdwn", + "text": f"{mention}\n*PR:* <{pr_url}|{pr_t} (#{pr_n})>\n*Findings:* {count}"}}, + {"type": "divider"}, + *report_blocks, + {"type": "context", "elements": [{"type": "mrkdwn", + "text": "This PR is *blocked* from merging until the findings are resolved. Push a new commit to re-run the review. Reply in thread for help."}]} + ], + } + + req = urllib.request.Request( + "https://slack.com/api/chat.postMessage", + data=json.dumps(payload).encode(), + headers={ + "Authorization": f"Bearer {token}", + "Content-Type": "application/json; charset=utf-8", + }, + method="POST", + ) + with urllib.request.urlopen(req) as resp: + body = json.loads(resp.read()) + if not body.get("ok"): + print("Slack post failed:", body, file=sys.stderr) + sys.exit(1) + PY + + - name: Fail check to block merge + if: steps.parse.outputs.has_findings == 'true' + env: + FINDINGS_COUNT: ${{ steps.parse.outputs.findings_count }} + run: | + echo "::error::Security review found ${FINDINGS_COUNT} issue(s). See Slack for details." + exit 1