Repository navigation
ci: add Claude security-review workflow for sensitive-path PRs #35715
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
c78f5ea
15ce17f
4d790b6
ef700d5
aab04c0
fe5a2f1
1d68602
9ae0942
d764ec0
a823c0e
4cad6cc
227b1c6
7933427
d69738a
c9d1a63
664f812
905f5d9
8c29ab8
591d6b2
d52f680
114d8ca
e1ed273
6455ef3
99fd5d3
128d814
1ab6bb2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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": <int, total kept findings>, | ||
| "high_count": <int>, | ||
| "medium_count": <int>, | ||
| "report_markdown": "<full markdown report>" | ||
| } | ||
|
|
||
| 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. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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)" | ||
|
Comment on lines
+140
to
+156
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This shouldn't be using the antrhopic api. should be bedrock so that we include it into our controls and commit leverage. Also consider using the dotcms/ai-workflows repo which allows for prompt injection to be passed across and gives you short lived OIDC and additional security controls AND the ability to pick from different models by variable.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done in On the |
||
|
|
||
| - 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 | ||
Uh oh!
There was an error while loading. Please reload this page.