diff --git a/.github/workflows/reusable-security-scan.yml b/.github/workflows/reusable-security-scan.yml index edb5963..d7da010 100644 --- a/.github/workflows/reusable-security-scan.yml +++ b/.github/workflows/reusable-security-scan.yml @@ -67,11 +67,13 @@ on: # Top-level permissions define the maximum permissions available to this workflow. # Job-level permissions further restrict as needed. +# pull-requests: write was removed in PPSC-556: PR commenting is now handled by +# the Armis backend GitHub App (armis-appsec[bot]) and no longer requires the +# workflow's GITHUB_TOKEN to have PR write access. permissions: contents: read security-events: write actions: read - pull-requests: write jobs: security-scan: @@ -82,7 +84,6 @@ jobs: contents: read security-events: write actions: read - pull-requests: write steps: - name: Checkout repository @@ -108,6 +109,7 @@ jobs: scan-timeout: ${{ inputs.scan-timeout }} include-files: ${{ inputs.include-files }} build-from-source: ${{ inputs.build-from-source }} + pr-comment: ${{ inputs.pr-comment && github.event_name == 'pull_request' }} continue-on-error: true - name: Ensure SARIF exists @@ -117,199 +119,10 @@ jobs: echo '{"version":"2.1.0","$schema":"https://raw.githubusercontent.com/oasis-tcs/sarif-spec/master/Schemata/sarif-schema-2.1.0.json","runs":[{"tool":{"driver":{"name":"armis-cli","version":"1.0.0"}},"results":[]}]}' > armis-results.sarif fi - - name: Post PR Comment with Results - if: always() && inputs.pr-comment && github.event_name == 'pull_request' - uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9 - with: - script: | - const fs = require('fs'); - - // Build a map of {filename: Set} for lines visible in the PR diff. - // These are the only lines GitHub Code Scanning annotates inline (added/context lines). - // Only '+' (added) and ' ' (context) lines are included; '-' (removed) lines are skipped - // because they don't exist in the new file. Lines starting with '\' (e.g. "\ No newline - // at end of file") are also skipped to avoid misaligning the computed line numbers. - function parseDiffLineNumbers(patch) { - const lines = new Set(); - if (!patch) return lines; - let currentLine = 0; - for (const raw of patch.split('\n')) { - const m = raw.match(/^@@ -\d+(?:,\d+)? \+(\d+)(?:,\d+)? @@/); - if (m) { currentLine = parseInt(m[1], 10); continue; } - if (currentLine === 0) continue; - if (raw.startsWith('-') || raw.startsWith('\\')) continue; - if (raw.startsWith('+') || raw.startsWith(' ')) { - lines.add(currentLine++); - } - } - return lines; - } - - // Use a null-prototype object to avoid prototype pollution from filenames - // like '__proto__' or 'constructor' that would corrupt a regular object. - // null sentinel means the file is changed but has no patch (e.g. large diffs - // or binary files) — findings for those files are always included so we never - // silently under-report issues on files GitHub couldn't supply a patch for. - const diffLines = Object.create(null); - let page = 1; - while (true) { - const { data: files } = await github.rest.pulls.listFiles({ - ...context.repo, - pull_number: context.issue.number, - per_page: 100, - page, - }); - if (!files.length) break; - for (const f of files) { - diffLines[f.filename] = f.patch ? parseDiffLineNumbers(f.patch) : null; - } - if (files.length < 100) break; - page++; - } - - // Read SARIF results - const sarif = JSON.parse(fs.readFileSync('armis-results.sarif', 'utf8')); - const allResults = sarif.runs?.[0]?.results || []; - - // Keep only findings whose (file, line) falls on a diff-visible line — - // these are the only ones GitHub Code Scanning will annotate inline. - // Findings for files with no patch data (null sentinel) are always kept - // to avoid under-reporting on large or binary files. - const results = allResults.filter(r => { - const file = r.locations?.[0]?.physicalLocation?.artifactLocation?.uri || ''; - const line = r.locations?.[0]?.physicalLocation?.region?.startLine; - if (!file || !line) return false; - if (!(file in diffLines)) return false; - const lineSet = diffLines[file]; - return lineSet === null || lineSet.has(line); - }); - - // Count by severity - read from properties.severity if available - const counts = { CRITICAL: 0, HIGH: 0, MEDIUM: 0, LOW: 0, INFO: 0 }; - for (const r of results) { - // Prefer properties.severity (set by armis-cli), fallback to level mapping - const severity = r.properties?.severity || { - 'error': 'HIGH', - 'warning': 'MEDIUM', - 'note': 'LOW', - 'none': 'INFO' - }[r.level || 'warning'] || 'INFO'; - counts[severity]++; - } - const total = results.length; - - // Build comment body - const marker = ''; - const status = counts.CRITICAL > 0 ? '🔴 CRITICAL issues found' : - counts.HIGH > 0 ? '🟠 HIGH issues found' : - total > 0 ? '🟡 Issues found' : '✅ No issues'; - - const workflowRef = process.env.GITHUB_WORKFLOW_REF || ''; - const ref = workflowRef.split('@')[1] || 'main'; - const logoLight = `https://raw.githubusercontent.com/ArmisSecurity/armis-cli/${ref}/docs/assets/appsec-logo-light.png`; - const logoDark = `https://raw.githubusercontent.com/ArmisSecurity/armis-cli/${ref}/docs/assets/appsec-logo-dark.png`; - // GitHub theme-aware images with height constraint (24px to match heading text) - const logo = `Armis AppSecArmis AppSec`; - - let body = `${marker}\n## ${logo} Security Scan Results\n\n**${status}**\n`; - if (total > 0) { - body += `\n| Severity | Count |\n|----------|-------|\n`; - if (counts.CRITICAL > 0) body += `| 🔴 CRITICAL | ${counts.CRITICAL} |\n`; - if (counts.HIGH > 0) body += `| 🟠 HIGH | ${counts.HIGH} |\n`; - if (counts.MEDIUM > 0) body += `| 🟡 MEDIUM | ${counts.MEDIUM} |\n`; - if (counts.LOW > 0) body += `| 🔵 LOW | ${counts.LOW} |\n`; - if (counts.INFO > 0) body += `| ⚪ INFO | ${counts.INFO} |\n`; - body += `\n**Total: ${total}**\n`; - } - - // Build detailed findings section - if (total > 0) { - body += `\n
View all ${total} findings\n\n`; - - // Group results by severity - const severityOrder = ['CRITICAL', 'HIGH', 'MEDIUM', 'LOW', 'INFO']; - const severityEmoji = { CRITICAL: '🔴', HIGH: '🟠', MEDIUM: '🟡', LOW: '🔵', INFO: '⚪' }; - - for (const severity of severityOrder) { - const severityResults = results.filter(r => - (r.properties?.severity || 'INFO') === severity - ); - - if (severityResults.length > 0) { - body += `### ${severityEmoji[severity]} ${severity} (${severityResults.length})\n\n`; - - for (const r of severityResults) { - const file = r.locations?.[0]?.physicalLocation?.artifactLocation?.uri || ''; - const line = r.locations?.[0]?.physicalLocation?.region?.startLine || ''; - const location = file ? (line ? `${file}:${line}` : file) : 'Unknown location'; - - // Parse title and description from message - const msgParts = (r.message?.text || '').split(': '); - const title = msgParts[0] || r.ruleId; - const description = msgParts.slice(1).join(': ') || ''; - - body += `
${r.ruleId} - ${title}\n\n`; - body += `**Location:** \`${location}\`\n\n`; - - if (description) { - body += `${description}\n\n`; - } - - // Code snippet - const snippet = r.properties?.codeSnippet; - if (snippet) { - body += '```\n' + snippet + '\n```\n\n'; - } - - // CVEs and CWEs - const cves = r.properties?.cves || []; - const cwes = r.properties?.cwes || []; - if (cves.length > 0) { - body += `**CVEs:** ${cves.join(', ')}\n\n`; - } - if (cwes.length > 0) { - body += `**CWEs:** ${cwes.join(', ')}\n\n`; - } - - // Package info - const pkg = r.properties?.package; - const version = r.properties?.version; - const fixVersion = r.properties?.fixVersion; - if (pkg) { - let pkgInfo = `**Package:** ${pkg}`; - if (version) pkgInfo += ` (${version})`; - if (fixVersion) pkgInfo += ` → Fix: ${fixVersion}`; - body += pkgInfo + '\n\n'; - } - - body += `
\n\n`; - } - } - } - - body += `
`; - } - - // Find and update existing comment, or create new - const { data: comments } = await github.rest.issues.listComments({ - ...context.repo, - issue_number: context.issue.number - }); - const existing = comments.find(c => c.body.includes(marker)); - - if (existing) { - await github.rest.issues.updateComment({ - ...context.repo, - comment_id: existing.id, - body - }); - } else { - await github.rest.issues.createComment({ - ...context.repo, - issue_number: context.issue.number, - body - }); - } + # PR commenting now happens inside the Armis action (Post Branded PR Comment + # step), which sends raw SARIF to the backend for server-side branded + # formatting (posts as armis-appsec[bot]). See action.yml. The previous + # ~130-line github-script formatter was removed in PPSC-556. - name: Upload SARIF to GitHub Code Scanning if: always() diff --git a/action.yml b/action.yml index 7de8daf..dc3a670 100644 --- a/action.yml +++ b/action.yml @@ -35,7 +35,7 @@ inputs: required: false default: 'sarif' fail-on: - description: 'Comma-separated severity levels to fail on (e.g., HIGH,CRITICAL). Empty string for informational mode (never fail).' + description: 'Comma-separated severity levels to fail on (e.g., HIGH,CRITICAL). Empty string (default) uses the CLI default (CRITICAL). Pass a value like HIGH,CRITICAL to override, or use the reusable workflow with fail-on: "" for informational mode.' required: false default: '' exit-code: @@ -64,6 +64,14 @@ inputs: description: 'Build CLI from source instead of downloading release (for testing)' required: false default: 'false' + pr-comment: + description: 'Post a branded PR comment via the Armis backend (requires the armis-appsec GitHub App installed on the repo). Only runs on pull_request events. Requires a persisted SARIF file: keep the default format (sarif) and set output-file (e.g. armis-results.sarif), otherwise the comment is skipped.' + required: false + default: 'false' + api-url: + description: 'Override the Armis API base URL (advanced; defaults to region-derived or production). Equivalent to ARMIS_API_URL.' + required: false + default: '' outputs: results: @@ -166,6 +174,7 @@ runs: INPUT_REGION: ${{ inputs.region }} INPUT_API_TOKEN: ${{ inputs.api-token }} INPUT_TENANT_ID: ${{ inputs.tenant-id }} + INPUT_API_URL: ${{ inputs.api-url }} SCAN_TYPE: ${{ inputs.scan-type }} SCAN_TARGET: ${{ inputs.scan-target }} OUTPUT_FORMAT: ${{ inputs.format }} @@ -185,6 +194,7 @@ runs: if [ -n "$INPUT_REGION" ]; then export ARMIS_REGION="$INPUT_REGION"; fi if [ -n "$INPUT_API_TOKEN" ]; then export ARMIS_API_TOKEN="$INPUT_API_TOKEN"; fi if [ -n "$INPUT_TENANT_ID" ]; then export ARMIS_TENANT_ID="$INPUT_TENANT_ID"; fi + if [ -n "$INPUT_API_URL" ]; then export ARMIS_API_URL="$INPUT_API_URL"; fi # Validate authentication: require JWT (client-id + client-secret) OR Basic (api-token + tenant-id) if [ -n "$ARMIS_CLIENT_ID" ] && [ -z "$ARMIS_CLIENT_SECRET" ]; then @@ -261,3 +271,196 @@ runs: echo "EOF" >> $GITHUB_OUTPUT exit $SCAN_EXIT_CODE + + - name: Post Branded PR Comment + # Only on pull requests when explicitly enabled. Requires the armis-appsec + # GitHub App installed on the target repo (server-side branded comment). + # always() so the comment is still posted when the scan step exits non-zero + # (e.g. findings exceeded --fail-on), matching the prior workflow behavior. + if: always() && inputs.pr-comment == 'true' && github.event_name == 'pull_request' + shell: bash + env: + INPUT_CLIENT_ID: ${{ inputs.client-id }} + INPUT_CLIENT_SECRET: ${{ inputs.client-secret }} + INPUT_API_TOKEN: ${{ inputs.api-token }} + INPUT_REGION: ${{ inputs.region }} + INPUT_API_URL: ${{ inputs.api-url }} + INPUT_OUTPUT_FILE: ${{ inputs.output-file }} + GITHUB_REPOSITORY: ${{ github.repository }} + GITHUB_EVENT_PATH: ${{ github.event_path }} + run: | + # Commenting must NEVER fail the build: soft-fail everywhere (warn + exit 0). + set +e + + # Escape %, CR, and LF before emitting a workflow command so that + # user-supplied input (region, backend detail text) cannot inject + # additional workflow commands into the log stream. + # bash parameter substitution operates on the full string (unlike sed, + # which is line-by-line and can't match embedded newlines portably). + warn() { + local msg="$1" + msg="${msg//%/%25}" + msg="${msg//$'\r'/%0D}" + msg="${msg//$'\n'/%0A}" + msg="${msg//::/%3A%3A}" + echo "::warning::Armis branded PR comment skipped: ${msg}" + exit 0 + } + + # Preflight: jq and curl must be available; missing tools should surface as a + # clear prerequisite error rather than a misleading SARIF-parsing warning. + for cmd in jq curl; do + command -v "$cmd" > /dev/null 2>&1 || warn "$cmd is required but not found on this runner" + done + + # Keep the request payload (full SARIF) and response private and transient: + # mktemp + an EXIT trap guarantees cleanup even on an early warn (exit 0), so + # these never linger in the workspace for later steps to pick up. + # armis:ignore cwe:377 reason:explicit XXXXXX template prevents name prediction; chmod 600 + EXIT trap ensure no exposure + PAYLOAD_FILE=$(mktemp "${TMPDIR:-/tmp}/armis-pr-comment.XXXXXX") + RESPONSE_FILE=$(mktemp "${TMPDIR:-/tmp}/armis-pr-response.XXXXXX") + chmod 600 "$PAYLOAD_FILE" "$RESPONSE_FILE" + trap 'rm -f "$PAYLOAD_FILE" "$RESPONSE_FILE"' EXIT + + # 1. Resolve API base URL: ARMIS_API_URL/api-url override -> region -> production. + # The base URL comes from operator-controlled action config (not PR content); + # we still require HTTPS for ALL base URLs so the credential sent below can + # never traverse plaintext. This is stricter than the CLI's NewAuthClient, + # which additionally permits http://localhost / http://127.0.0.1 for local + # development — an exception that has no use case in CI. + # armis:ignore cwe:918 reason:BASE_URL is operator config (api-url/ARMIS_API_URL/region), HTTPS-enforced; not derived from untrusted request data + BASE_URL="${INPUT_API_URL:-${ARMIS_API_URL:-}}" + # Fall back to ARMIS_REGION env var when inputs.region is empty, matching CLI env var behavior. + EFFECTIVE_REGION="${INPUT_REGION:-${ARMIS_REGION:-}}" + if [ -z "$BASE_URL" ]; then + if [ -n "$EFFECTIVE_REGION" ]; then + # Explicit allowlist mirroring internal/auth.RegionalBaseURL. + # Interpolating the region into the hostname would produce wrong + # endpoints (e.g. moose.eu1.armis.com instead of eu.moose.armis.com) + # and would also allow an attacker-controlled region to redirect + # credentials to an arbitrary host (CWE-918). + # Mirroring internal/auth.RegionalBaseURL: only eu1 has a distinct + # data plane; all other valid region codes (us1, au1, future ones) + # fall through to the primary host rather than failing — matching + # the CLI's graceful fallback behavior and avoiding silently + # skipping comments for au1 tenants. + case "$EFFECTIVE_REGION" in + eu1) BASE_URL="https://eu.moose.armis.com" ;; + *) BASE_URL="https://moose.armis.com" ;; + esac + else + BASE_URL="https://moose.armis.com" + fi + fi + BASE_URL="${BASE_URL%/}" + case "$BASE_URL" in + https://*) ;; + # armis:ignore cwe:918 reason:BASE_URL is operator config (api-url/ARMIS_API_URL/region), not attacker-reachable; this case IS the HTTPS guard that prevents SSRF — warn and exit 0 + *) warn "API base URL must use HTTPS (got '${BASE_URL}')" ;; + esac + + # 2. Locate the SARIF file produced by the scan step. + SARIF_FILE="${INPUT_OUTPUT_FILE:-armis-results.sarif}" + if [ ! -s "$SARIF_FILE" ]; then + warn "SARIF file '$SARIF_FILE' is missing or empty — pr-comment needs a persisted SARIF file (keep format: sarif and set output-file)" + fi + if ! jq -e '.runs' "$SARIF_FILE" > /dev/null 2>&1; then + warn "SARIF file '$SARIF_FILE' has no 'runs' key" + fi + + # 3. Resolve repo and PR number (composite actions lack github.event.* context). + REPO="$GITHUB_REPOSITORY" + if ! echo "$REPO" | grep -Eq '^[A-Za-z0-9._-]+/[A-Za-z0-9._-]+$'; then + warn "unexpected repository name '$REPO'" + fi + PR_NUMBER=$(jq -r '.pull_request.number // empty' "$GITHUB_EVENT_PATH" 2>/dev/null) + if ! echo "$PR_NUMBER" | grep -Eq '^[1-9][0-9]*$'; then + warn "could not determine pull request number" + fi + + # 4. Build the Authorization header. + # Prefer JWT (exchange client-id/secret); fall back to Basic api-token. + AUTH="" + if [ -n "$INPUT_CLIENT_ID" ] && [ -n "$INPUT_CLIENT_SECRET" ]; then + # Write the credential payload to a private file and send it via @file so the + # client secret never appears on the curl command line. Secret is also kept out + # of jq's argv by using $ENV.INPUT_CLIENT_SECRET (read from process env, not arg). + # armis:ignore cwe:377 reason:explicit XXXXXX template prevents name prediction; chmod 600 + immediate rm ensure no exposure + TOKEN_REQ_FILE=$(mktemp "${TMPDIR:-/tmp}/armis-token-req.XXXXXX") + chmod 600 "$TOKEN_REQ_FILE" + # Use $ENV.INPUT_CLIENT_SECRET so the secret is read from the process + # environment (not placed in jq's argv, which is readable via /proc/*/cmdline). + # armis:ignore cwe:522 reason:secret accessed via $ENV (not argv); result written to chmod-600 temp file, consumed via @file, rm'd immediately after use + # armis:ignore cwe:522 reason:chmod-600 temp file; secret via $ENV (not argv); consumed via curl @file (never on cmdline); rm'd immediately after curl + jq -n \ + --arg id "$INPUT_CLIENT_ID" \ + --arg region "${EFFECTIVE_REGION:-}" \ + 'if $region != "" then {client_id:$id, client_secret:$ENV.INPUT_CLIENT_SECRET, region:$region} else {client_id:$id, client_secret:$ENV.INPUT_CLIENT_SECRET} end' \ + > "$TOKEN_REQ_FILE" + TOKEN_RESP_FILE=$(mktemp "${TMPDIR:-/tmp}/armis-token-resp.XXXXXX") + chmod 600 "$TOKEN_RESP_FILE" + # armis:ignore cwe:377 reason:explicit XXXXXX template; chmod 600 + rm after use + TOKEN_HTTP=$(curl -sS --max-time 30 -o "$TOKEN_RESP_FILE" -w '%{http_code}' \ + -X POST "${BASE_URL}/api/v1/auth/token" \ + -H 'Content-Type: application/json' --data-binary @"$TOKEN_REQ_FILE") + TOKEN_CURL_EXIT=$? + rm -f "$TOKEN_REQ_FILE" + if [ "$TOKEN_CURL_EXIT" -ne 0 ]; then + rm -f "$TOKEN_RESP_FILE" + warn "JWT token exchange network error (curl exit $TOKEN_CURL_EXIT)" + fi + if [ "$TOKEN_HTTP" -lt 200 ] || [ "$TOKEN_HTTP" -ge 300 ]; then + TOKEN_DETAIL=$(jq -r '.detail // empty' "$TOKEN_RESP_FILE" 2>/dev/null) + rm -f "$TOKEN_RESP_FILE" + warn "JWT token exchange failed HTTP $TOKEN_HTTP${TOKEN_DETAIL:+ ($TOKEN_DETAIL)}" + fi + JWT=$(jq -r '.token // empty' "$TOKEN_RESP_FILE" 2>/dev/null) + rm -f "$TOKEN_RESP_FILE" + if [ -z "$JWT" ]; then + warn "JWT token exchange failed: no token in response (check client-id/client-secret)" + fi + AUTH="Bearer ${JWT}" + elif [ -n "$INPUT_API_TOKEN" ]; then + # Strip CR/LF to prevent HTTP header injection before constructing the + # Authorization header — a token accidentally containing a newline could + # inject additional headers into the curl request. + SANITIZED_TOKEN="${INPUT_API_TOKEN//$'\r'/}" + SANITIZED_TOKEN="${SANITIZED_TOKEN//$'\n'/}" + # Backend compares the value after 'Basic ' verbatim (not base64-encoded), + # matching the CLI's own scheme; base64-encoding would break authentication. + # Sent over HTTPS only (enforced above); never logged. + # armis:ignore cwe:522 reason:Basic scheme requires the raw token in the header per the backend contract (verbatim hmac compare); sent over HTTPS, never echoed + AUTH="Basic ${SANITIZED_TOKEN}" + else + warn "no credentials provided (set client-id/client-secret or api-token)" + fi + + # 5. Build the request payload safely (no string interpolation of SARIF). + if ! jq -n --arg repo "$REPO" --argjson pr "$PR_NUMBER" --slurpfile sarif "$SARIF_FILE" \ + '{repo:$repo, pr_number:$pr, sarif:$sarif[0]}' > "$PAYLOAD_FILE" 2>/dev/null; then + warn "failed to build request payload" + fi + + # 6. POST to the branded-comment webhook. + HTTP_CODE=$(curl -sS --max-time 60 -o "$RESPONSE_FILE" -w '%{http_code}' \ + -X POST "${BASE_URL}/api/v1/webhook/pr-comment" \ + -H "Authorization: ${AUTH}" \ + -H 'Content-Type: application/json' \ + --data-binary @"$PAYLOAD_FILE") + CURL_EXIT=$? + + if [ "$CURL_EXIT" -ne 0 ]; then + warn "request to backend failed (curl exit $CURL_EXIT)" + fi + if [ "$HTTP_CODE" -lt 200 ] || [ "$HTTP_CODE" -ge 300 ]; then + DETAIL=$(jq -r '.detail // empty' "$RESPONSE_FILE" 2>/dev/null) + warn "backend returned HTTP $HTTP_CODE${DETAIL:+ ($DETAIL)}" + fi + + # armis:ignore cwe:20 reason:COMMENT_URL is only echoed to log (never used in commands/API calls); CR, LF, and '::' stripped to prevent workflow-command injection + COMMENT_URL=$(jq -r '.comment_url // empty' "$RESPONSE_FILE" 2>/dev/null) + # Strip CR, LF, and '::' so a backend-injected newline can't inject workflow commands. + COMMENT_URL="${COMMENT_URL//$'\r'/}" + COMMENT_URL="${COMMENT_URL//$'\n'/}" + COMMENT_URL="${COMMENT_URL//::/%3A%3A}" + echo "Branded PR comment posted: ${COMMENT_URL:-ok}" diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index cf6a190..63a5fb5 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -10,15 +10,20 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - Documentation: an SSO Setup Guide for IT admins (`docs/SSO-SETUP.md`) covering the end-to-end workflow — registering `armis-cli` as an OIDC app in IdP, and deploying to developer machines via MDM. +- GitHub Action: new `pr-comment` input posts a branded PR comment (as `armis-appsec[bot]`) by sending the scan's SARIF to the Armis backend for server-side formatting. Requires the `armis-appsec` GitHub App installed on the repository. Authentication reuses existing `client-id`/`client-secret` (JWT) or `api-token` (Basic) credentials, and commenting failures never fail the build. (PPSC-556) +- GitHub Action: new `api-url` input to override the Armis API base URL (equivalent to `ARMIS_API_URL`). (PPSC-556) ### Changed - `auth setup`: now auto-detects your tenant ID and region from your Armis credentials — the detected tenant seeds the interactive prompt, and the region routes the registration request to the correct regional host. On success the command prints the exact environment variables to deploy to developer machines. +- Reusable security-scan workflow now delegates PR commenting to the Armis action's branded backend flow instead of formatting comments in-workflow, giving consistent branding across the bot-assignment and Action integrations. (PPSC-556) ### Deprecated ### Removed +- Reusable security-scan workflow: removed the ~130-line `actions/github-script` block that formatted PR comments client-side (superseded by server-side branded comments). (PPSC-556) + ### Fixed ### Security diff --git a/docs/CI-INTEGRATION.md b/docs/CI-INTEGRATION.md index 5ca2481..5b4ab2c 100644 --- a/docs/CI-INTEGRATION.md +++ b/docs/CI-INTEGRATION.md @@ -124,7 +124,6 @@ on: permissions: contents: read security-events: write - pull-requests: write jobs: security-scan: @@ -143,8 +142,8 @@ jobs: |-------|------|---------|-------------| | `scan-type` | string | `repo` | Type of scan: `repo` or `image` | | `scan-target` | string | `.` | Path for repo scan, image name for image scan | -| `fail-on` | string | `CRITICAL` | Comma-separated severity levels to fail on (e.g., `HIGH,CRITICAL`). Set to empty string to never fail. | -| `pr-comment` | boolean | `true` | Post scan results as PR comment | +| `fail-on` | string | `''` | Comma-separated severity levels to fail on (e.g., `HIGH,CRITICAL`). Empty string (default) means informational mode — never fail on findings. | +| `pr-comment` | boolean | `true` | Post a branded PR comment (as `armis-appsec[bot]`) via the Armis backend on `pull_request` events. Ignored on push/schedule/other triggers. Requires the [armis-appsec GitHub App](#branded-pr-comments) installed on the repo; otherwise the comment is silently skipped. | | `upload-artifact` | boolean | `true` | Upload SARIF results as artifact | | `artifact-retention-days` | number | `30` | Days to retain artifacts | | `image-tarball` | string | | Path to image tarball (for image scans) | @@ -168,13 +167,14 @@ jobs: permissions: contents: read # Read repository content security-events: write # Upload SARIF to Code Scanning - pull-requests: write # Post PR comments actions: read # Access workflow artifacts +# Note: pull-requests: write is NOT required. PR commenting is handled by +# the armis-appsec GitHub App backend — no GITHUB_TOKEN PR write needed. ``` #### What You Get -**PR Comments**: Detailed breakdown of findings by severity with expandable details for each issue: +**Branded PR Comments**: A detailed breakdown of findings by severity with expandable details for each issue, posted as `armis-appsec[bot]`: | Severity | Count | |----------|-------| @@ -182,6 +182,9 @@ permissions: | HIGH | 5 | | MEDIUM | 12 | + +> **How it works:** The action sends the raw SARIF results to the Armis backend, which formats and posts the comment server-side (the same formatter used by the `armis-appsec` bot when assigned to a PR). This keeps formatting consistent across integrations and lets it improve without an action release. It requires the **armis-appsec GitHub App** to be installed on the repository — without it the backend cannot post as the bot and the comment is skipped (the scan itself still runs and uploads results). Authentication reuses your existing `client-id`/`client-secret` (JWT) or `api-token` (legacy) credentials; commenting failures never fail the build. + **GitHub Code Scanning**: Findings appear in the Security tab, inline in PR diffs, and as check annotations. **Artifacts**: SARIF results are stored for the configured retention period, enabling historical analysis. @@ -244,7 +247,7 @@ jobs: | `api-token` | No* | | Armis API token (legacy) | | `tenant-id` | No* | | Tenant identifier (legacy, not needed with JWT) | | `format` | No | `sarif` | Output format: `human`, `json`, `sarif`, `junit` | -| `fail-on` | No | `CRITICAL` | Severity levels to fail on | +| `fail-on` | No | `''` | Severity levels to fail on. Empty string (default) uses the CLI default (`CRITICAL`). Set explicitly (e.g. `HIGH,CRITICAL`) to override. | | `exit-code` | No | `1` | Exit code when failing | | `no-progress` | No | `true` | Disable progress indicators | | `image-tarball` | No | | Path to image tarball (image scans) | @@ -252,6 +255,8 @@ jobs: | `scan-timeout` | No | `60` | Timeout in minutes | | `include-files` | No | | Comma-separated file paths to scan | | `build-from-source` | No | `false` | Build from source (testing) | +| `pr-comment` | No | `false` | Post a branded PR comment (as `armis-appsec[bot]`) via the Armis backend. Only runs on `pull_request` events and requires the [armis-appsec GitHub App](#branded-pr-comments) installed on the repo. Needs a persisted SARIF file — keep the default `format: sarif` and set `output-file` (e.g. `armis-results.sarif`), or the comment is skipped. | +| `api-url` | No | | Override the Armis API base URL (advanced; defaults to region-derived or production). Equivalent to `ARMIS_API_URL`. | \* One authentication method is required: either `client-id` + `client-secret` (recommended) or `api-token` + `tenant-id` (legacy). @@ -319,7 +324,6 @@ on: permissions: contents: read security-events: write - pull-requests: write jobs: get-changed-files: