From 67c7ec2ac0308d85f8909b79278274765ed82ab8 Mon Sep 17 00:00:00 2001 From: KaifAhmad1 Date: Sun, 2 Aug 2026 22:36:54 +0530 Subject: [PATCH] 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: " 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
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. --- .github/workflows/security-scan.yml | 149 ++++++++++++++++------------ SECURITY.md | 2 +- 2 files changed, 86 insertions(+), 65 deletions(-) diff --git a/.github/workflows/security-scan.yml b/.github/workflows/security-scan.yml index 1260ed5b..4c537a98 100644 --- a/.github/workflows/security-scan.yml +++ b/.github/workflows/security-scan.yml @@ -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" @@ -118,73 +125,87 @@ jobs: with: script: | const fs = require('fs'); - - // Read safety report - let safetyResults = ''; - 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'; + + // 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
block so the comment + // doesn't turn into a wall of text. + function renderSection(title, reportPath, parse) { + let data; + try { + data = JSON.parse(fs.readFileSync(reportPath, 'utf8')); + } catch (e) { + return [ + `### ${title}`, + `⚠️ No report found at \`${reportPath}\` — the scan may have failed before producing output. Check the job logs.`, + ].join('\n'); } - } catch (e) { - safetyResults = '## Safety scan completed\\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`; - }); - } else { - banditResults = '## No High Severity Security Issues Found\\n'; - } - } else { - banditResults = '## No Bandit Issues Found\\n'; + + const items = parse(data); + if (items.length === 0) { + return [`### ${title}`, `✅ No findings.`].join('\n'); } - } catch (e) { - banditResults = '## Bandit scan completed\\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`; - } + + const lines = [`### ${title}`, `Found **${items.length}**.`, '']; + const shown = items.slice(0, 15); + if (items.length > 15) { + lines.push('
', 'Show all findings', ''); + lines.push(...items); + lines.push('', '
'); } else { - semgrepResults = '## No Security Patterns Found\\n'; + lines.push(...shown); } - } catch (e) { - semgrepResults = '## Semgrep scan completed\\n'; + return lines.join('\n'); } - - // 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.`; - - // Post comment with error handling + + 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'}` + ) + ); + + 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'); + 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) { diff --git a/SECURITY.md b/SECURITY.md index afccd454..303ba07e 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -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.