Skip to content

[PPSC-556] feat(action): post branded PR comments via backend webhook #230

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.

Sign up for GitHub

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

Open
yiftach-armis wants to merge 3 commits into
base: main
Choose a base branch
from
Open

[PPSC-556] feat(action): post branded PR comments via backend webhook #230

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5b87cb8
feat(action): post branded PR comments via backend webhook (PPSC-556)
yiftach-armis Jun 18, 2026
99c9c97
fix(action): wire api-url into scan step; correct HTTPS comment (PPSC…
yiftach-armis Jun 18, 2026
1db6fbb
fix(action): document SARIF requirement for pr-comment; tidy temp fil…
yiftach-armis Jun 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
198 changes: 5 additions & 193 deletions .github/workflows/reusable-security-scan.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,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
Expand All @@ -117,199 +118,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@v9
with:
script: |
const fs = require('fs');

// Build a map of {filename: Set<lineNumber>} 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 = '<!-- armis-security-scan -->';
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 = `<img alt="Armis AppSec" src="${logoLight}#gh-dark-mode-only" height="24"><img alt="Armis AppSec" src="${logoDark}#gh-light-mode-only" height="24">`;

let body = `${marker}\n## ${logo}&nbsp;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<details><summary>View all ${total} findings</summary>\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 += `<details><summary><code>${r.ruleId}</code> - ${title}</summary>\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 += `</details>\n\n`;
}
}
}

body += `</details>`;
}

// 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()
Expand Down
143 changes: 143 additions & 0 deletions action.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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: ''
Comment thread
yiftach-armis marked this conversation as resolved.

outputs:
results:
Expand Down Expand Up @@ -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 }}
Expand All @@ -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
Expand Down Expand Up @@ -261,3 +271,136 @@ 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

warn() { echo "::warning::Armis branded PR comment skipped: $1"; exit 0; }

# 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.
PAYLOAD_FILE=$(mktemp)
RESPONSE_FILE=$(mktemp)
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:-}}"
if [ -z "$BASE_URL" ]; then
if [ -n "$INPUT_REGION" ]; then
if echo "$INPUT_REGION" | grep -Eq '^[a-z0-9]([a-z0-9-]*[a-z0-9])?$'; then
BASE_URL="https://moose.${INPUT_REGION}.armis.com"
else
warn "invalid region '$INPUT_REGION'"
fi
else
BASE_URL="https://moose.armis.com"
fi
fi
BASE_URL="${BASE_URL%/}"
# armis:ignore cwe:918 reason:BASE_URL is operator config (api-url/ARMIS_API_URL/region), HTTPS-enforced here; not derived from untrusted request data
case "$BASE_URL" in
https://*) ;;
*) 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 (process list / /proc).
TOKEN_REQ_FILE=$(mktemp)
chmod 600 "$TOKEN_REQ_FILE"
if [ -n "$INPUT_REGION" ]; then
jq -n --arg id "$INPUT_CLIENT_ID" --arg secret "$INPUT_CLIENT_SECRET" --arg region "$INPUT_REGION" \
'{client_id:$id, client_secret:$secret, region:$region}' > "$TOKEN_REQ_FILE"
else
jq -n --arg id "$INPUT_CLIENT_ID" --arg secret "$INPUT_CLIENT_SECRET" \
'{client_id:$id, client_secret:$secret}' > "$TOKEN_REQ_FILE"
fi
TOKEN_RESP=$(curl -sS --max-time 30 -X POST "${BASE_URL}/api/v1/auth/token" \
-H 'Content-Type: application/json' --data-binary @"$TOKEN_REQ_FILE")
rm -f "$TOKEN_REQ_FILE"
JWT=$(echo "$TOKEN_RESP" | jq -r '.token // empty' 2>/dev/null)
if [ -z "$JWT" ]; then
warn "JWT token exchange failed (check client-id/client-secret)"
fi
AUTH="Bearer ${JWT}"
elif [ -n "$INPUT_API_TOKEN" ]; then
# 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 ${INPUT_API_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

COMMENT_URL=$(jq -r '.comment_url // empty' "$RESPONSE_FILE" 2>/dev/null)
echo "Branded PR comment posted: ${COMMENT_URL:-ok}"
Loading
Loading