mirror of
https://github.com/nlohmann/json.git
synced 2026-07-21 19:23:03 +04:00
Harden CI: validate PR artifact inputs, migrate off deprecated Semgrep action (#5232)
* Harden CI workflows: validate PR artifact inputs and migrate off deprecated Semgrep action Address two CI/supply-chain hardening items from the 2026-07-03 security audit: - comment_check_amalgamation.yml (todo 117): the privileged `workflow_run` job consumes an untrusted PR artifact. Validate `author` against a strict GitHub-username pattern and `number` as a positive integer before use, and extract the artifact into a dedicated directory (`unzip -o pr.zip -d ./pr_artifact`), reading only the two expected files by fixed path. This prevents Markdown/mention injection via the attacker-controlled `author` text and avoids a malicious archive touching the workspace. - semgrep.yml (todo 118): `returntocorp/semgrep-action` is deprecated (the org was renamed to `semgrep/*`). Replace it with an explicit `semgrep ci` invocation via the maintained CLI; the deployment is inferred from SEMGREP_APP_TOKEN. Todo 116 (CIFuzz `@master` refs) already carries a comment documenting the OSS-Fuzz-recommended exception, so no change is needed there. Signed-off-by: Niels Lohmann <mail@nlohmann.me> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Fix Semgrep step: use `semgrep scan` instead of token-gated `semgrep ci` The CI `Scan` job failed with "Path does not exist: semgrep.sarif" because `semgrep ci` requires a login token (SEMGREP_APP_TOKEN), which this repo does not have configured, so it bailed without producing a SARIF file. The former returntocorp/semgrep-action, given no token, fell back to plain `semgrep scan --sarif`; match that with `semgrep scan --config auto`, which needs no token and always produces the SARIF for upload. Signed-off-by: Niels Lohmann <mail@nlohmann.me> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Signed-off-by: Niels Lohmann <mail@nlohmann.me> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -47,7 +47,10 @@ jobs:
|
|||||||
|
|
||||||
var hasPatch = artifacts.data.artifacts.some((artifact) => artifact.name == "amalgamation-patch");
|
var hasPatch = artifacts.data.artifacts.some((artifact) => artifact.name == "amalgamation-patch");
|
||||||
core.setOutput('has_patch', String(hasPatch));
|
core.setOutput('has_patch', String(hasPatch));
|
||||||
- run: unzip pr.zip
|
# Extract the untrusted PR artifact into a dedicated empty directory and
|
||||||
|
# read only the two expected files by fixed path afterwards. This avoids a
|
||||||
|
# malicious archive overwriting workspace files or escaping via ../ paths.
|
||||||
|
- run: unzip -o pr.zip -d ./pr_artifact
|
||||||
|
|
||||||
- name: 'Comment on PR'
|
- name: 'Comment on PR'
|
||||||
uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0
|
uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0
|
||||||
@@ -55,8 +58,19 @@ jobs:
|
|||||||
github-token: ${{ secrets.GITHUB_TOKEN }}
|
github-token: ${{ secrets.GITHUB_TOKEN }}
|
||||||
script: |
|
script: |
|
||||||
var fs = require('fs');
|
var fs = require('fs');
|
||||||
const author = fs.readFileSync('./author')
|
// Both values come from a fork-triggered workflow and are therefore
|
||||||
const issue_number = Number(fs.readFileSync('./number'));
|
// attacker-controlled. Validate them strictly before use to prevent
|
||||||
|
// Markdown/mention injection and bogus REST API filters.
|
||||||
|
const author = fs.readFileSync('./pr_artifact/author', 'utf8').trim();
|
||||||
|
if (!/^[A-Za-z0-9-]{1,39}$/.test(author)) {
|
||||||
|
core.setFailed(`Refusing to proceed: untrusted author value '${author}' is not a valid GitHub username.`);
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
const issue_number = Number(fs.readFileSync('./pr_artifact/number', 'utf8').trim());
|
||||||
|
if (!Number.isInteger(issue_number) || issue_number <= 0) {
|
||||||
|
core.setFailed('Refusing to proceed: untrusted PR number is not a positive integer.');
|
||||||
|
return;
|
||||||
|
}
|
||||||
const opts = github.rest.issues.listForRepo.endpoint.merge({
|
const opts = github.rest.issues.listForRepo.endpoint.merge({
|
||||||
owner: context.repo.owner,
|
owner: context.repo.owner,
|
||||||
repo: context.repo.repo,
|
repo: context.repo.repo,
|
||||||
|
|||||||
@@ -41,12 +41,23 @@ jobs:
|
|||||||
with:
|
with:
|
||||||
persist-credentials: false
|
persist-credentials: false
|
||||||
|
|
||||||
# Scan code using project's configuration on https://semgrep.dev/manage
|
# The former returntocorp/semgrep-action is deprecated (the org was renamed
|
||||||
- uses: returntocorp/semgrep-action@713efdd345f3035192eaa63f56867b88e63e4e5d
|
# to semgrep/*); the maintained approach is to install the CLI and invoke
|
||||||
with:
|
# it directly. We use `semgrep scan` (not `semgrep ci`, which requires a
|
||||||
publishToken: ${{ secrets.SEMGREP_APP_TOKEN }}
|
# login token): with no SEMGREP_APP_TOKEN configured this is exactly what
|
||||||
publishDeployment: ${{ secrets.SEMGREP_DEPLOYMENT_ID }}
|
# the old action fell back to, running community rules with no token.
|
||||||
generateSarif: "1"
|
# SEMGREP_APP_TOKEN is still passed through so registry auth works if a
|
||||||
|
# token is ever added.
|
||||||
|
- name: Install Semgrep
|
||||||
|
run: python3 -m pip install --user semgrep
|
||||||
|
|
||||||
|
# `semgrep scan --sarif` always exits 0 even with findings; continue-on-error
|
||||||
|
# is a safety net so the SARIF upload still runs if the scan itself errors.
|
||||||
|
- name: Run Semgrep
|
||||||
|
run: semgrep scan --config auto --sarif --output=semgrep.sarif
|
||||||
|
continue-on-error: true
|
||||||
|
env:
|
||||||
|
SEMGREP_APP_TOKEN: ${{ secrets.SEMGREP_APP_TOKEN }}
|
||||||
|
|
||||||
# Upload SARIF file generated in previous step
|
# Upload SARIF file generated in previous step
|
||||||
- name: Upload SARIF file
|
- name: Upload SARIF file
|
||||||
|
|||||||
Reference in New Issue
Block a user