mirror of
https://github.com/semantica-agi/semantica.git
synced 2026-08-29 04:26:20 +00:00
fix: repair broken Safety scan and PR comment formatting
The "Comment PR with Security Results" step was producing garbled output (literal \n characters instead of newlines, "undefined:" labels) because: - Every line in the JS comment builder used \n (escaped backslash-n) inside template literals, which JS renders as the literal two-character string \n, not a newline. - The Semgrep section read issue.rule_id, but Semgrep's JSON field is check_id - hence "undefined: <path>" for every entry. Rewrote the comment builder to construct each section as an array of lines joined with a real '\n', with correct field names, and collapsed long finding lists into a <details> block instead of a flat list. Verified by extracting the exact script and running it under node against synthetic fixtures matching each tool's real JSON schema (found/clean/ missing-report paths all render correctly). While tracing the "undefined" and always-empty Safety section, found the Safety step itself was silently broken: - `safety check --json --output safety-report.json` is invalid in Safety 3.x: --output now selects a console format (json/text/screen), not a file path. The command errored on every run (swallowed by `|| true`), so safety-report.json was never created and the PR comment always fell back to a generic "scan completed" message. Switched to `--save-json`, which is the correct flag for writing a JSON report to disk, and confirmed against the real safety 3.8.1 CLI locally. - Even with a report, the code read vuln.package - the real field is package_name. - The job never installed Semantica's own dependencies before scanning, so `safety check` (which defaults to scanning the environment) was auditing the scanner tools' own dependencies, not Semantica's. Added `pip install -e ".[llm-litellm]"` so the project's actual dependency tree - including the LiteLLM extra this whole hardening effort is about - is what gets scanned. Also updated the corresponding SECURITY.md bullet to describe what Safety actually covers now.
This commit is contained in:
KaifAhmad1
@@ -46,21 +46,28 @@ jobs:
|
||||
run: |
|
||||
python -m pip install --upgrade pip
|
||||
pip install safety bandit semgrep jq
|
||||
# Install the project itself (core deps + the LiteLLM provider extra)
|
||||
# so Safety scans Semantica's actual dependency tree, not just the
|
||||
# scanner tools' own dependencies.
|
||||
pip install -e ".[llm-litellm]"
|
||||
|
||||
- name: Run Safety Check (Package Vulnerabilities)
|
||||
run: |
|
||||
safety check --json --output safety-report.json || true
|
||||
# NOTE: Safety 3.x repurposed --output to select a console format
|
||||
# (json/text/screen/...), not a file path. Writing JSON to a file
|
||||
# now requires --save-json; the previous `--output safety-report.json`
|
||||
# usage was silently invalid and never produced a report.
|
||||
safety check --save-json safety-report.json || true
|
||||
echo "Checking for package vulnerabilities..."
|
||||
|
||||
# Count vulnerabilities safely
|
||||
VULNS=$(safety check --json --output /dev/stdout 2>/dev/null | jq '.vulnerabilities | length' 2>/dev/null || echo "0")
|
||||
VULNS=$(jq '.vulnerabilities | length' safety-report.json 2>/dev/null || echo "0")
|
||||
|
||||
if [ "$VULNS" -gt 0 ]; then
|
||||
echo "❌ Security vulnerabilities found: $VULNS"
|
||||
echo "CI will fail to prevent merging of vulnerable dependencies"
|
||||
echo ""
|
||||
echo "Vulnerability details:"
|
||||
safety check || true
|
||||
jq -r '.vulnerabilities[] | "- \(.package_name)==\(.analyzed_version): \(.vulnerability_id) (\(.CVE // "no CVE assigned"))"' safety-report.json || true
|
||||
exit 1
|
||||
else
|
||||
echo "✅ No security vulnerabilities found"
|
||||
@@ -119,72 +126,86 @@ jobs:
|
||||
script: |
|
||||
const fs = require('fs');
|
||||
|
||||
// Read safety report
|
||||
let safetyResults = '';
|
||||
// Renders one tool's findings as a section. `items` is already
|
||||
// the list of pre-formatted "- `thing` in `where`" strings; this
|
||||
// just handles the found/not-found/report-missing framing and
|
||||
// collapses long lists into a <details> block so the comment
|
||||
// doesn't turn into a wall of text.
|
||||
function renderSection(title, reportPath, parse) {
|
||||
let data;
|
||||
try {
|
||||
const safetyData = JSON.parse(fs.readFileSync('safety-report.json', 'utf8'));
|
||||
if (safetyData.vulnerabilities && safetyData.vulnerabilities.length > 0) {
|
||||
safetyResults = `## Safety Vulnerabilities Found\\n`;
|
||||
safetyData.vulnerabilities.forEach(vuln => {
|
||||
safetyResults += `- **${vuln.package}**: ${vuln.advisory}\\n`;
|
||||
});
|
||||
} else {
|
||||
safetyResults = '## No Safety Vulnerabilities Found\\n';
|
||||
}
|
||||
data = JSON.parse(fs.readFileSync(reportPath, 'utf8'));
|
||||
} catch (e) {
|
||||
safetyResults = '## Safety scan completed\\n';
|
||||
return [
|
||||
`### ${title}`,
|
||||
`⚠️ No report found at \`${reportPath}\` — the scan may have failed before producing output. Check the job logs.`,
|
||||
].join('\n');
|
||||
}
|
||||
|
||||
// Read bandit report
|
||||
let banditResults = '';
|
||||
try {
|
||||
const banditData = JSON.parse(fs.readFileSync('bandit-report.json', 'utf8'));
|
||||
if (banditData.results && banditData.results.length > 0) {
|
||||
const highIssues = banditData.results.filter(issue => issue.issue_severity === 'HIGH');
|
||||
if (highIssues.length > 0) {
|
||||
banditResults = `## High Severity Security Issues Found\\n`;
|
||||
highIssues.forEach(issue => {
|
||||
banditResults += `- **${issue.test_name}**: ${issue.filename}:${issue.line_number}\\n`;
|
||||
});
|
||||
const items = parse(data);
|
||||
if (items.length === 0) {
|
||||
return [`### ${title}`, `✅ No findings.`].join('\n');
|
||||
}
|
||||
|
||||
const lines = [`### ${title}`, `Found **${items.length}**.`, ''];
|
||||
const shown = items.slice(0, 15);
|
||||
if (items.length > 15) {
|
||||
lines.push('<details>', '<summary>Show all findings</summary>', '');
|
||||
lines.push(...items);
|
||||
lines.push('', '</details>');
|
||||
} else {
|
||||
banditResults = '## No High Severity Security Issues Found\\n';
|
||||
lines.push(...shown);
|
||||
}
|
||||
} else {
|
||||
banditResults = '## No Bandit Issues Found\\n';
|
||||
}
|
||||
} catch (e) {
|
||||
banditResults = '## Bandit scan completed\\n';
|
||||
return lines.join('\n');
|
||||
}
|
||||
|
||||
// Read semgrep report
|
||||
let semgrepResults = '';
|
||||
try {
|
||||
const semgrepData = JSON.parse(fs.readFileSync('semgrep-report.json', 'utf8'));
|
||||
if (semgrepData.results && semgrepData.results.length > 0) {
|
||||
semgrepResults = `## Security Patterns Found\\n`;
|
||||
semgrepData.results.slice(0, 10).forEach(issue => {
|
||||
semgrepResults += `- **${issue.rule_id}**: ${issue.path}\\n`;
|
||||
});
|
||||
if (semgrepData.results.length > 10) {
|
||||
semgrepResults += `- ... and ${semgrepData.results.length - 10} more\\n`;
|
||||
}
|
||||
} else {
|
||||
semgrepResults = '## No Security Patterns Found\\n';
|
||||
}
|
||||
} catch (e) {
|
||||
semgrepResults = '## Semgrep scan completed\\n';
|
||||
}
|
||||
const safetySection = renderSection(
|
||||
'Safety — dependency vulnerabilities',
|
||||
'safety-report.json',
|
||||
(data) => (data.vulnerabilities || []).map(
|
||||
(v) => `- \`${v.package_name}==${v.analyzed_version}\`: ${v.vulnerability_id}` +
|
||||
(v.CVE ? ` (${v.CVE})` : '') + ` — ${v.advisory || 'no advisory text'}`
|
||||
)
|
||||
);
|
||||
|
||||
// Create summary comment
|
||||
const comment = `# 🔒 Security Scan Results\\n\\n${safetyResults}\\n\\n${banditResults}\\n\\n${semgrepResults}\\n\\n---\\n\\n*This security scan runs automatically on source-code PRs and bi-weekly (skipped for doc/markdown-only changes).*\\n\\n📊 **Security Policy**: CI fails on vulnerabilities and HIGH severity issues.`;
|
||||
const banditSection = renderSection(
|
||||
'Bandit — HIGH-severity code issues',
|
||||
'bandit-report.json',
|
||||
(data) => (data.results || [])
|
||||
.filter((issue) => issue.issue_severity === 'HIGH')
|
||||
.map((issue) => `- \`${issue.test_name}\` in \`${issue.filename}:${issue.line_number}\``)
|
||||
);
|
||||
|
||||
const semgrepSection = renderSection(
|
||||
'Semgrep — static analysis patterns',
|
||||
'semgrep-report.json',
|
||||
(data) => (data.results || []).map(
|
||||
(issue) => `- \`${issue.check_id}\` in \`${issue.path}:${issue.start?.line ?? '?'}\``
|
||||
)
|
||||
);
|
||||
|
||||
const comment = [
|
||||
'# 🔒 Security Scan Results',
|
||||
'',
|
||||
safetySection,
|
||||
'',
|
||||
banditSection,
|
||||
'',
|
||||
semgrepSection,
|
||||
'',
|
||||
'---',
|
||||
'',
|
||||
'*This security scan runs automatically on source-code PRs and bi-weekly (skipped for doc/markdown-only changes).*',
|
||||
'',
|
||||
'📊 **Security Policy**: CI fails on Safety vulnerabilities and Bandit HIGH-severity findings. Semgrep findings above are informational and do not block merge.',
|
||||
].join('\n');
|
||||
|
||||
// Post comment with error handling
|
||||
try {
|
||||
await github.rest.issues.createComment({
|
||||
issue_number: context.issue.number,
|
||||
owner: context.repo.owner,
|
||||
repo: context.repo.repo,
|
||||
body: comment
|
||||
body: comment,
|
||||
});
|
||||
console.log('✅ Security comment posted successfully');
|
||||
} catch (error) {
|
||||
|
||||
+1
-1
@@ -164,7 +164,7 @@ Every scan below runs continuously in CI, not just at release time:
|
||||
- **CodeQL** (`security-and-quality` query pack) — Python source: injection, unsafe deserialization, and other code-level vulnerability classes. Runs in `codeql.yml` on every push/PR to `main` and weekly.
|
||||
- **Bandit** — Python-specific security anti-patterns (hardcoded secrets, unsafe `eval`/`pickle`, weak crypto, etc.); CI fails on any HIGH-severity finding. Runs in `security-scan.yml` on every push/PR to `main` and twice weekly.
|
||||
- **Semgrep** (`p/security` ruleset) — cross-language static-analysis security patterns. Runs in `security-scan.yml` on every push/PR to `main` and twice weekly.
|
||||
- **Safety** — known CVEs in installed Python dependencies; CI fails on any match. Runs in `security-scan.yml` on every push/PR to `main` and twice weekly.
|
||||
- **Safety** — known CVEs in Semantica's own installed dependencies, including optional LLM-provider extras such as LiteLLM; CI fails on any match. Runs in `security-scan.yml` on every push/PR to `main` and twice weekly.
|
||||
- **pip-audit** — independent, PyPA-maintained vulnerability database cross-check against installed dependencies (Safety and pip-audit use different advisory sources, so both run). Runs in `security.yml` weekly.
|
||||
- **Microsoft Defender for DevOps** (`eslint`, `templateanalyzer`, `terrascan`) — JavaScript/TypeScript lint-security rules and infrastructure-as-code misconfigurations. Runs in `defender-for-devops.yml` on every push/PR to `main` and weekly.
|
||||
- **Checkov** — Kubernetes, Helm, Dockerfile, GitHub Actions, and secrets-pattern IaC scanning; results upload to the same Security tab as CodeQL. Runs in `defender-for-devops.yml` on every push/PR to `main` and weekly.
|
||||
|
||||
Reference in New Issue
Block a user