[PPSC-556] feat(action): post branded PR comments via backend webhook#230
Open
yiftach-armis wants to merge 14 commits into
Open
[PPSC-556] feat(action): post branded PR comments via backend webhook#230yiftach-armis wants to merge 14 commits into
yiftach-armis wants to merge 14 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR moves PR commenting for the Armis security scan from client-side formatting in the reusable workflow to a server-side, branded comment flow by sending SARIF to an Armis backend webhook (posting as armis-appsec[bot]).
Changes:
- Added
pr-commentandapi-urlinputs to the GitHub Action and implemented a new “Post Branded PR Comment” step that posts SARIF to the backend webhook (soft-fail by design). - Updated the reusable workflow to delegate PR commenting to the action (removing the prior
actions/github-scriptformatter). - Updated docs and changelog to describe branded PR comments and the new inputs / requirements.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
docs/CI-INTEGRATION.md |
Documents branded PR commenting behavior and adds pr-comment / api-url to input references. |
docs/CHANGELOG.md |
Records the new Action inputs and the workflow change/removal of the old formatter. |
action.yml |
Adds new inputs and implements the backend webhook PR comment posting logic. |
.github/workflows/reusable-security-scan.yml |
Routes PR commenting through the action and removes the in-workflow formatter block. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
yiftach-armis
force-pushed
the
feat/PPSC-556-branded-pr-comments
branch
from
June 18, 2026 09:39
4ae70b0 to
5b87cb8
Compare
Test Coverage Reporttotal: (statements) 73.1% Coverage by function |
yiftach-armis
added a commit
that referenced
this pull request
Jun 18, 2026
…-556) Address Copilot review on PR #230: - api-url is now exported as ARMIS_API_URL for the scan step, matching its documented 'Equivalent to ARMIS_API_URL' contract (previously the input only reached the PR-comment webhook step). - Correct the PR-comment HTTPS comment: the action enforces HTTPS for all base URLs, which is intentionally stricter than the CLI's NewAuthClient (which permits http://localhost for local dev).
yiftach-armis
added a commit
that referenced
this pull request
Jun 18, 2026
…es (PPSC-556) Address Copilot review round 2 on PR #230: - Document that pr-comment requires a persisted SARIF file (keep format: sarif and set output-file); clarify in the input description, the missing-SARIF warning, and the CI-INTEGRATION docs so enabling pr-comment without output-file doesn't appear to silently do nothing. - Write the request payload and backend response to mktemp files with an EXIT trap instead of leaving pr-comment-payload.json / -response.json in the workspace, so the SARIF payload stays private and is cleaned up even on early soft-fail exits (matches the token-request file pattern).
yiftach-armis
added a commit
that referenced
this pull request
Jul 6, 2026
…-556) Address Copilot review on PR #230: - api-url is now exported as ARMIS_API_URL for the scan step, matching its documented 'Equivalent to ARMIS_API_URL' contract (previously the input only reached the PR-comment webhook step). - Correct the PR-comment HTTPS comment: the action enforces HTTPS for all base URLs, which is intentionally stricter than the CLI's NewAuthClient (which permits http://localhost for local dev).
yiftach-armis
added a commit
that referenced
this pull request
Jul 6, 2026
…es (PPSC-556) Address Copilot review round 2 on PR #230: - Document that pr-comment requires a persisted SARIF file (keep format: sarif and set output-file); clarify in the input description, the missing-SARIF warning, and the CI-INTEGRATION docs so enabling pr-comment without output-file doesn't appear to silently do nothing. - Write the request payload and backend response to mktemp files with an EXIT trap instead of leaving pr-comment-payload.json / -response.json in the workspace, so the SARIF payload stays private and is cleaned up even on early soft-fail exits (matches the token-request file pattern).
yiftach-armis
force-pushed
the
feat/PPSC-556-branded-pr-comments
branch
from
July 6, 2026 07:38
1db6fbb to
6493163
Compare
Send raw SARIF to the Armis backend (POST /api/v1/webhook/pr-comment) for server-side branded PR comments posted as armis-appsec[bot], instead of formatting comments client-side in the workflow. - action.yml: add pr-comment and api-url inputs plus a Post Branded PR Comment step. Resolves the API base URL (api-url/ARMIS_API_URL -> region -> production, HTTPS-enforced), exchanges client-id/secret for a JWT (Bearer) with Basic api-token fallback, builds the request body with jq, and POSTs the SARIF. Soft-fails everywhere (warn + exit 0) so commenting never fails the build. - reusable-security-scan.yml: delegate commenting to the action; remove the ~130-line github-script formatter block. - docs: document the new inputs, the armis-appsec App requirement, and changelog entries.
…-556) Address Copilot review on PR #230: - api-url is now exported as ARMIS_API_URL for the scan step, matching its documented 'Equivalent to ARMIS_API_URL' contract (previously the input only reached the PR-comment webhook step). - Correct the PR-comment HTTPS comment: the action enforces HTTPS for all base URLs, which is intentionally stricter than the CLI's NewAuthClient (which permits http://localhost for local dev).
…es (PPSC-556) Address Copilot review round 2 on PR #230: - Document that pr-comment requires a persisted SARIF file (keep format: sarif and set output-file); clarify in the input description, the missing-SARIF warning, and the CI-INTEGRATION docs so enabling pr-comment without output-file doesn't appear to silently do nothing. - Write the request payload and backend response to mktemp files with an EXIT trap instead of leaving pr-comment-payload.json / -response.json in the workspace, so the SARIF payload stays private and is cleaned up even on early soft-fail exits (matches the token-request file pattern).
yiftach-armis
force-pushed
the
feat/PPSC-556-branded-pr-comments
branch
from
July 12, 2026 12:24
6493163 to
01592cb
Compare
…ing; correct fail-on docs (PPSC-556) - warn(): escape %, CR, LF before ::warning:: to prevent workflow-command injection - Region URL: replace interpolation with explicit allowlist (eu1→eu.moose.armis.com, us1→moose.armis.com) matching internal/auth.RegionalBaseURL — fixes wrong endpoint for eu1 and prevents CWE-918 redirect - docs/CI-INTEGRATION.md: correct two fail-on default entries from CRITICAL to '' (informational mode)
Mirrors internal/auth.RegionalBaseURL: au1 and any future region codes resolve to moose.armis.com instead of failing with a warning, so branded PR comments are not silently skipped for au1 tenants.
…tus (PPSC-556) - Add command-availability preflight for jq and curl so missing tools give a clear prerequisite error rather than a misleading SARIF-parsing warning - Token exchange now captures curl exit code + HTTP status code (matching the webhook POST pattern), so network errors and non-2xx responses surface the specific problem instead of a generic credential hint - Collapse two-branch jq credential write into one call to reduce attack surface
…omment_url log output (PPSC-556) - docs/CI-INTEGRATION.md: note that pr-comment only fires on pull_request events - action.yml: strip CR, LF, '::' from comment_url before echoing to prevent workflow-command injection from a backend-controlled response value
…ault CRITICAL (PPSC-556) The action description and docs incorrectly stated that an empty fail-on input enables 'informational mode (never fail)'. In reality, when fail-on is empty the action omits --fail-on entirely, so the CLI falls back to its built-in default (CRITICAL). Correct both places to reflect actual behavior.
… on HTTPS fail (PPSC-556) - reusable-security-scan.yml: remove pull-requests:write from top-level and job-level permissions — PR commenting now runs via armis-appsec[bot] backend and never uses the workflow's GITHUB_TOKEN for write access (least privilege) - docs/CI-INTEGRATION.md: remove pull-requests:write from all example permission blocks and add a note explaining the backend App handles commenting - action.yml: log actual BASE_URL in HTTPS enforcement warning so operators can diagnose misconfiguration directly
…SC-556) Replace --arg secret with $ENV.INPUT_CLIENT_SECRET so the secret is read from the process environment rather than jq's argument vector. On Linux, /proc/PID/cmdline is readable by the same runner user, making --arg an unnecessary exposure vector; environment variables are not visible there.
…SC-556) When inputs.region is empty, use ARMIS_REGION env var as fallback so the pr-comment step uses the same region as the scan step (which already exports ARMIS_REGION). Fixes incorrect base URL and missing region in the JWT token-exchange payload when region is configured via env var.
…PPSC-556) A newline in INPUT_API_TOKEN (e.g. from copy/paste) could inject additional HTTP headers into the curl request. Strip carriage return and newline before constructing the Basic authorization header.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related Issue
Type of Change
Problem
PR comments were formatted client-side in the reusable workflow via a ~130-line
actions/github-scriptblock, posted as the generic GitHub Actions bot — branding and formatting drifted from thearmis-appsecbot used elsewhere, and any formatting change required an action release.Solution
The action now sends raw SARIF to the Armis backend (
POST /api/v1/webhook/pr-comment) for server-side branded comments posted asarmis-appsec[bot].action.ymlgainspr-commentandapi-urlinputs plus a Post Branded PR Comment step that resolves the HTTPS-enforced API base URL, exchangesclient-id/client-secretfor a JWT (with Basicapi-tokenfallback), and POSTs the SARIF — soft-failing everywhere so commenting never breaks the build. The reusable workflow delegates to the action and drops its github-script formatter, and the docs/CHANGELOG document the new inputs and thearmis-appsecApp requirement.Testing
Automated Tests
Manual Testing
The change is GitHub Actions YAML and embedded shell; behavior is exercised at runtime in CI. No Go code changed, so the suite is a regression sanity check.
Reviewer Notes
Focus on the shell in the new
action.ymlstep: API-URL resolution + HTTPS enforcement, JWT-vs-Basic auth selection, and the soft-fail (warn +exit 0) paths. Commenting requires thearmis-appsecGitHub App installed on the repo; without it the backend skips the comment while the scan still runs and uploads results.Checklist