From 5178c989bc09d1a3c8c5cf451ab68727246c1c9b Mon Sep 17 00:00:00 2001 From: Geoff Hutchison Date: Wed, 25 Mar 2026 22:12:32 -0400 Subject: [PATCH 1/4] Add a security scan and use /validate and /security comment triggers Suggestions on steps from Claude Code (earning credit line) Co-authored-by: Claude Signed-off-by: Geoff Hutchison --- .github/avogadro-security.yaml | 159 ++++++++ .github/workflows/security-scan.yml | 574 ++++++++++++++++++++++++++++ .github/workflows/validate-pr.yml | 35 +- 3 files changed, 765 insertions(+), 3 deletions(-) create mode 100644 .github/avogadro-security.yaml create mode 100644 .github/workflows/security-scan.yml diff --git a/.github/avogadro-security.yaml b/.github/avogadro-security.yaml new file mode 100644 index 0000000..df46041 --- /dev/null +++ b/.github/avogadro-security.yaml @@ -0,0 +1,159 @@ +rules: + # ── Code execution / injection ── + - id: avogadro-eval-exec + patterns: + - pattern-either: + - pattern: eval(...) + - pattern: exec(...) + - pattern: compile(..., ..., "exec") + message: > + eval()/exec()/compile() found. These allow arbitrary code execution + and are almost never needed in Avogadro plugins. Use structured data + processing (JSON, etc.) instead. + severity: ERROR + languages: [python] + + - id: avogadro-dynamic-import + patterns: + - pattern-either: + - pattern: __import__(...) + - pattern: importlib.import_module(...) + message: > + Dynamic import detected. Plugins should use standard imports. + If loading optional dependencies, use try/except around normal imports. + severity: WARNING + languages: [python] + + # ── Deserialization ── + - id: avogadro-unsafe-deserialization + patterns: + - pattern-either: + - pattern: pickle.load(...) + - pattern: pickle.loads(...) + - pattern: _pickle.load(...) + - pattern: marshal.load(...) + - pattern: marshal.loads(...) + - pattern: yaml.load(...) # without Loader=SafeLoader + - pattern: shelve.open(...) + message: > + Unsafe deserialization can execute arbitrary code. Use JSON or + other safe formats. If YAML is needed, use yaml.safe_load(). + severity: ERROR + languages: [python] + + # ── Subprocess calls ── + - id: avogadro-subprocess-shell + patterns: + - pattern-either: + - pattern: subprocess.call(..., shell=True, ...) + - pattern: subprocess.run(..., shell=True, ...) + - pattern: subprocess.Popen(..., shell=True, ...) + - pattern: os.system(...) + - pattern: os.popen(...) + message: > + Shell command execution detected. Prefer subprocess with shell=False + and explicit argument lists. os.system() and os.popen() should never + be used. + severity: ERROR + languages: [python] + + - id: avogadro-subprocess-review + patterns: + - pattern-either: + - pattern: subprocess.call(...) + - pattern: subprocess.run(...) + - pattern: subprocess.Popen(...) + - pattern: subprocess.check_output(...) + - pattern: subprocess.check_call(...) + message: > + Subprocess call detected. This is often legitimate for plugins that + wrap external tools (ORCA, Gaussian, xtb, etc.), but verify the + command is not constructed from untrusted input. + severity: WARNING + languages: [python] + + # ── Network access ── + - id: avogadro-network-access + patterns: + - pattern-either: + - pattern: requests.get(...) + - pattern: requests.post(...) + - pattern: requests.put(...) + - pattern: requests.delete(...) + - pattern: urllib.request.urlopen(...) + - pattern: http.client.HTTPConnection(...) + - pattern: http.client.HTTPSConnection(...) + - pattern: socket.socket(...) + message: > + Network access detected. Verify the plugin genuinely needs network + access and that URLs are not constructed from untrusted input. + severity: WARNING + languages: [python] + + # ── File system operations ── + - id: avogadro-dangerous-file-ops + patterns: + - pattern-either: + - pattern: shutil.rmtree(...) + - pattern: os.removedirs(...) + - pattern: pathlib.Path(...).unlink(...) + message: > + Destructive file operations detected. Verify paths are restricted + to the plugin's working directory. + severity: WARNING + languages: [python] + + - id: avogadro-path-traversal + patterns: + - pattern-either: + - pattern: open($PATH + ..., ...) + - pattern: open(f"...{$VAR}...", ...) + - pattern: os.path.join(..., $USER_INPUT, ...) + message: > + Possible path traversal — file paths should be validated to prevent + accessing files outside the intended directory. + severity: WARNING + languages: [python] + + # ── Credentials and secrets ── + - id: avogadro-hardcoded-secret + patterns: + - pattern-either: + - pattern: $VAR = "sk-..." + - pattern: $VAR = "api_key_..." + - pattern: | + password = "..." + - pattern: | + token = "..." + - pattern: | + secret = "..." + - pattern: | + api_key = "..." + message: > + Possible hardcoded credential. Use environment variables or + Avogadro's configuration system for secrets. + severity: ERROR + languages: [python] + + # ── Native code / FFI ── + - id: avogadro-native-code + patterns: + - pattern-either: + - pattern: ctypes.cdll.LoadLibrary(...) + - pattern: ctypes.CDLL(...) + - pattern: cffi.FFI() + message: > + Native code loading via ctypes/cffi. This bypasses Python's safety + features and requires careful review. + severity: ERROR + languages: [python] + + # ── Avogadro-specific best practices ── + - id: avogadro-stderr-for-progress + patterns: + - pattern: print(...) # in the context of a plugin script + message: > + Avogadro plugin scripts communicate via stdout (JSON). Use + sys.stderr.write() for progress messages, not print(). + severity: INFO + languages: [python] diff --git a/.github/workflows/security-scan.yml b/.github/workflows/security-scan.yml new file mode 100644 index 0000000..ccdfff5 --- /dev/null +++ b/.github/workflows/security-scan.yml @@ -0,0 +1,574 @@ +# .github/workflows/security-scan.yml +# +# Triggered by PRs that touch repositories.toml, or by commenting "/security" +# on a PR. +# For each added or modified plugin: +# 1. Clones the plugin repo at the specified commit +# 2. Runs Bandit (Python security linter) +# 3. Runs Semgrep with community + custom rules +# 4. Runs pip-audit on declared dependencies +# 5. Checks for dangerous patterns (subprocess, eval, network, etc.) +# 6. Generates a structured security report as a PR comment + +name: Security Scan + +on: + pull_request: + paths: + - "repositories.toml" + issue_comment: + types: [created] + +permissions: + contents: read + pull-requests: write + +jobs: + # ──────────────────────────────────────────── + # Phase 1: Identify which plugins changed + # ──────────────────────────────────────────── + detect-changes: + runs-on: ubuntu-latest + # Run on PR events, or on "/security" comments on PRs + if: >- + github.event_name == 'pull_request' || + (github.event_name == 'issue_comment' && + github.event.issue.pull_request && + contains(github.event.comment.body, '/security')) + outputs: + plugins: ${{ steps.diff.outputs.plugins }} + count: ${{ steps.diff.outputs.count }} + pr_number: ${{ steps.pr.outputs.number }} + steps: + - name: Get PR details + id: pr + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: | + if [ "${{ github.event_name }}" = "pull_request" ]; then + echo "number=${{ github.event.pull_request.number }}" >> "$GITHUB_OUTPUT" + echo "head_sha=${{ github.event.pull_request.head.sha }}" >> "$GITHUB_OUTPUT" + echo "base_sha=${{ github.event.pull_request.base.sha }}" >> "$GITHUB_OUTPUT" + else + pr_number="${{ github.event.issue.number }}" + echo "number=$pr_number" >> "$GITHUB_OUTPUT" + pr_data=$(gh api repos/${{ github.repository }}/pulls/$pr_number) + head_sha=$(echo "$pr_data" | jq -r '.head.sha') + base_sha=$(echo "$pr_data" | jq -r '.base.sha') + echo "head_sha=$head_sha" >> "$GITHUB_OUTPUT" + echo "base_sha=$base_sha" >> "$GITHUB_OUTPUT" + fi + + - name: Checkout PR head + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + with: + ref: ${{ steps.pr.outputs.head_sha }} + path: head + + - name: Checkout base branch + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + with: + ref: ${{ steps.pr.outputs.base_sha }} + path: base + + - uses: actions/setup-python@a309ff8b426b58ec0e2a45f0f869d46889d02405 # v6.2.0 + with: + python-version: "3.12" + + - name: Identify changed plugins + id: diff + run: | + diff_result=$(python head/scripts/parse_plugins.py diff \ + base/repositories.toml head/repositories.toml) + + # Build a matrix of plugins that need scanning (added + modified) + plugins=$(echo "$diff_result" | python3 -c " + import json, sys + d = json.load(sys.stdin) + scan_list = [] + for p in d['added']: + scan_list.append({ + 'name': p['name'], + 'repo': p['repo'], + 'commit': p['commit'], + 'path': p.get('path', '.'), + 'plugin_type': p.get('plugin_type', 'pypkg'), + 'change_type': 'added' + }) + for p in d['modified']: + if p.get('new_commit'): + scan_list.append({ + 'name': p['name'], + 'repo': p['repo'], + 'commit': p['new_commit'], + 'path': p.get('path', '.'), + 'plugin_type': p.get('plugin_type', 'pypkg'), + 'change_type': 'modified' + }) + print(json.dumps(scan_list)) + ") + + count=$(echo "$plugins" | python3 -c "import json,sys; print(len(json.load(sys.stdin)))") + echo "plugins=$(echo "$plugins" | jq -c .)" >> "$GITHUB_OUTPUT" + echo "count=$count" >> "$GITHUB_OUTPUT" + + # ──────────────────────────────────────────── + # Phase 2: Scan each plugin (parallel matrix) + # ──────────────────────────────────────────── + scan-plugin: + needs: detect-changes + if: needs.detect-changes.outputs.count != '0' + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + plugin: ${{ fromJson(needs.detect-changes.outputs.plugins) }} + steps: + - name: Checkout this repo (for custom rules) + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + with: + path: _this + + - name: Clone plugin at pinned commit + id: clone + run: | + git clone "${{ matrix.plugin.repo }}" plugin-src + cd plugin-src + git checkout "${{ matrix.plugin.commit }}" + + # Determine the actual source directory + src_path="${{ matrix.plugin.path }}" + if [ "$src_path" != "." ] && [ -d "$src_path" ]; then + echo "src_dir=plugin-src/$src_path" >> "$GITHUB_OUTPUT" + else + echo "src_dir=plugin-src" >> "$GITHUB_OUTPUT" + fi + + - uses: actions/setup-python@a309ff8b426b58ec0e2a45f0f869d46889d02405 # v6.2.0 + with: + python-version: "3.12" + + - name: Install scanning tools + run: | + pip install bandit pip-audit safety semgrep + + # ── Scan 1: Bandit (Python security linter) ── + - name: Run Bandit + id: bandit + continue-on-error: true + run: | + src="${{ steps.clone.outputs.src_dir }}" + echo "### Bandit Results" > bandit-report.md + + # Run bandit, capture output + bandit -r "$src" \ + -f json \ + -ll \ + --exclude '**/test*,**/.git' \ + -o bandit-results.json 2>/dev/null || true + + # Parse results + python3 -c " + import json + with open('bandit-results.json') as f: + data = json.load(f) + + results = data.get('results', []) + high = [r for r in results if r['issue_severity'] == 'HIGH'] + medium = [r for r in results if r['issue_severity'] == 'MEDIUM'] + low = [r for r in results if r['issue_severity'] == 'LOW'] + + with open('bandit-report.md', 'w') as f: + f.write('### 🔍 Bandit (Python Security Linter)\n\n') + if not results: + f.write('✅ No issues found.\n') + else: + f.write(f'| Severity | Count |\n') + f.write(f'|----------|-------|\n') + f.write(f'| 🔴 High | {len(high)} |\n') + f.write(f'| 🟡 Medium | {len(medium)} |\n') + f.write(f'| 🟢 Low | {len(low)} |\n\n') + + if high or medium: + f.write('
Details (high/medium)\n\n') + for r in high + medium: + f.write(f\"\"\"- **{r['issue_severity']}**: {r['issue_text']} + - File: \`{r['filename']}:{r['line_number']}\` + - CWE: {r.get('issue_cwe', {}).get('id', 'N/A')} + - Test: {r['test_id']} + \"\"\") + f.write('\n
\n') + + # Set exit code + exit(1 if high else 0) + " || echo "bandit_failed=true" >> "$GITHUB_OUTPUT" + + # ── Scan 2: Dangerous pattern grep ── + - name: Check for dangerous patterns + id: patterns + run: | + src="${{ steps.clone.outputs.src_dir }}" + echo "### 🔎 Dangerous Pattern Check" > patterns-report.md + + found_critical=false + found_review=false + + # ── Critical patterns (almost always bad in plugins) ── + echo "" >> patterns-report.md + echo "#### Critical Patterns" >> patterns-report.md + + declare -A critical_patterns + critical_patterns=( + ["eval("]="eval() — arbitrary code execution" + ["exec("]="exec() — arbitrary code execution" + ["compile("]="compile() — dynamic code compilation" + ["__import__"]="__import__() — dynamic import (bypass restrictions)" + ["importlib"]="importlib — dynamic module loading" + ["pickle.load"]="pickle.loads — unsafe deserialization" + ["marshal.load"]="marshal — unsafe deserialization" + ["shelve.open"]="shelve — pickle-backed storage" + ["os.system("]="os.system() — shell command execution" + ["ctypes"]="ctypes — native code access" + ["cffi"]="cffi — native code access" + ) + + for pattern in "${!critical_patterns[@]}"; do + desc="${critical_patterns[$pattern]}" + matches=$(grep -rn --include="*.py" "$pattern" "$src" 2>/dev/null | head -5 || true) + if [ -n "$matches" ]; then + found_critical=true + echo "" >> patterns-report.md + echo "🔴 **$desc**" >> patterns-report.md + echo '```' >> patterns-report.md + echo "$matches" >> patterns-report.md + echo '```' >> patterns-report.md + fi + done + + if [ "$found_critical" = "false" ]; then + echo "✅ No critical patterns found." >> patterns-report.md + fi + + # ── Review patterns (legitimate but need human eyes) ── + echo "" >> patterns-report.md + echo "#### Patterns Requiring Review" >> patterns-report.md + + declare -A review_patterns + review_patterns=( + ["subprocess"]="subprocess — runs external programs" + ["requests."]="requests library — network access" + ["urllib"]="urllib — network access" + ["http.client"]="http.client — network access" + ["socket"]="socket — raw network access" + ["open("]="file I/O — check paths are safe" + ["shutil.rmtree"]="shutil.rmtree — recursive deletion" + ["os.remove"]="os.remove — file deletion" + ["tempfile"]="tempfile — temp file creation" + ["webbrowser"]="webbrowser — opens URLs in browser" + ) + + for pattern in "${!review_patterns[@]}"; do + desc="${review_patterns[$pattern]}" + matches=$(grep -rn --include="*.py" "$pattern" "$src" 2>/dev/null | \ + grep -v "^Binary" | head -3 || true) + if [ -n "$matches" ]; then + found_review=true + echo "" >> patterns-report.md + echo "🟡 **$desc**" >> patterns-report.md + echo '```' >> patterns-report.md + echo "$matches" >> patterns-report.md + echo '```' >> patterns-report.md + fi + done + + if [ "$found_review" = "false" ]; then + echo "✅ No review-worthy patterns found." >> patterns-report.md + fi + + echo "critical=$found_critical" >> "$GITHUB_OUTPUT" + echo "review=$found_review" >> "$GITHUB_OUTPUT" + + # ── Scan 3: Semgrep (community + custom rules) ── + - name: Run Semgrep + id: semgrep + continue-on-error: true + run: | + src="${{ steps.clone.outputs.src_dir }}" + echo "### 🛡️ Semgrep Analysis" > semgrep-report.md + + # Run with community Python security rules + semgrep scan \ + --config "p/python" \ + --config "p/security-audit" \ + --config "p/secrets" \ + --json \ + --output semgrep-results.json \ + "$src" 2>/dev/null || true + + # Also run custom Avogadro rules if they exist + if [ -f "_this/.github/avogadro-security.yaml" ]; then + semgrep scan \ + --config "_this/.github/avogadro-security.yaml" \ + --json \ + --output semgrep-custom-results.json \ + "$src" 2>/dev/null || true + fi + + python3 -c " + import json, os + + all_results = [] + for fname in ['semgrep-results.json', 'semgrep-custom-results.json']: + if os.path.exists(fname): + with open(fname) as f: + data = json.load(f) + all_results.extend(data.get('results', [])) + + with open('semgrep-report.md', 'w') as f: + f.write('### 🛡️ Semgrep Analysis\n\n') + if not all_results: + f.write('✅ No issues found.\n') + else: + # Group by severity + by_severity = {} + for r in all_results: + sev = r.get('extra', {}).get('severity', 'INFO') + by_severity.setdefault(sev, []).append(r) + + f.write(f'Found **{len(all_results)}** issue(s):\n\n') + f.write('| Severity | Count |\n') + f.write('|----------|-------|\n') + for sev in ['ERROR', 'WARNING', 'INFO']: + items = by_severity.get(sev, []) + if items: + icon = {'ERROR': '🔴', 'WARNING': '🟡', 'INFO': 'ℹ️'}.get(sev, '⚪') + f.write(f'| {icon} {sev} | {len(items)} |\n') + + f.write('\n
Issue details\n\n') + for r in all_results: + sev = r.get('extra', {}).get('severity', 'INFO') + msg = r.get('extra', {}).get('message', 'No message') + path = r.get('path', '?') + line = r.get('start', {}).get('line', '?') + rule = r.get('check_id', '?') + f.write(f'- **[{sev}]** \`{path}:{line}\` — {msg} ({rule})\n') + f.write('\n
\n') + " + + # ── Scan 4: Dependency audit ── + - name: Audit dependencies + id: deps + continue-on-error: true + run: | + src="${{ steps.clone.outputs.src_dir }}" + echo "### 📦 Dependency Audit" > deps-report.md + + # Look for dependency declarations + has_deps=false + dep_file="" + + if [ -f "$src/pyproject.toml" ]; then + dep_file="$src/pyproject.toml" + has_deps=true + elif [ -f "$src/requirements.txt" ]; then + dep_file="$src/requirements.txt" + has_deps=true + elif [ -f "$src/setup.py" ]; then + dep_file="$src/setup.py" + has_deps=true + fi + + if [ "$has_deps" = "false" ]; then + echo "ℹ️ No dependency file found (pyproject.toml, requirements.txt, setup.py)." >> deps-report.md + echo "vuln_count=0" >> "$GITHUB_OUTPUT" + exit 0 + fi + + echo "Dependency file: \`$dep_file\`" >> deps-report.md + echo "" >> deps-report.md + + # pip-audit + echo "#### pip-audit" >> deps-report.md + if [ -f "$src/requirements.txt" ]; then + pip-audit -r "$src/requirements.txt" --format json --output pip-audit.json 2>/dev/null || true + elif [ -f "$src/pyproject.toml" ]; then + # Create a temporary venv and install to audit + python -m venv /tmp/audit-venv + source /tmp/audit-venv/bin/activate + pip install "$src" 2>/dev/null || pip install -e "$src" 2>/dev/null || true + pip-audit --format json --output pip-audit.json 2>/dev/null || true + deactivate + fi + + if [ -f "pip-audit.json" ]; then + python3 -c " + import json + with open('pip-audit.json') as f: + data = json.load(f) + + deps = data if isinstance(data, list) else data.get('dependencies', []) + vulns = [d for d in deps if d.get('vulns')] + + with open('deps-report.md', 'a') as f: + if not vulns: + f.write('✅ No known vulnerabilities in dependencies.\n') + else: + f.write(f'⚠️ **{len(vulns)} package(s) with known vulnerabilities:**\n\n') + for d in vulns: + f.write(f\"\"\"- **{d['name']}** {d.get('version', '?')}:\n\"\"\") + for v in d['vulns']: + f.write(f\"\"\" - {v.get('id', '?')}: {v.get('description', 'N/A')[:100]}\n\"\"\") + + print(len(vulns)) + " > /tmp/vuln_count + echo "vuln_count=$(cat /tmp/vuln_count)" >> "$GITHUB_OUTPUT" + else + echo "⚠️ Could not run pip-audit." >> deps-report.md + echo "vuln_count=0" >> "$GITHUB_OUTPUT" + fi + + # Check for non-PyPI dependency sources + echo "" >> deps-report.md + echo "#### Dependency Source Check" >> deps-report.md + suspicious=$(grep -rn --include="*.toml" --include="*.txt" --include="*.cfg" \ + -E "(git\+|https?://(?!pypi)|file://)" "$src" 2>/dev/null | head -10 || true) + if [ -n "$suspicious" ]; then + echo "⚠️ **Non-PyPI dependency sources found** (review carefully):" >> deps-report.md + echo '```' >> deps-report.md + echo "$suspicious" >> deps-report.md + echo '```' >> deps-report.md + else + echo "✅ All dependencies appear to use standard PyPI sources." >> deps-report.md + fi + + # ── Scan 5: Code metrics & overview ── + - name: Code overview + run: | + src="${{ steps.clone.outputs.src_dir }}" + echo "### 📊 Code Overview" > overview-report.md + + # Count Python files and lines + py_files=$(find "$src" -name "*.py" -not -path "*/.git/*" | wc -l) + py_lines=$(find "$src" -name "*.py" -not -path "*/.git/*" -exec cat {} + 2>/dev/null | wc -l) + + echo "| Metric | Value |" >> overview-report.md + echo "|--------|-------|" >> overview-report.md + echo "| Python files | $py_files |" >> overview-report.md + echo "| Total Python lines | $py_lines |" >> overview-report.md + echo "| Plugin type | ${{ matrix.plugin.plugin_type }} |" >> overview-report.md + echo "| Change type | ${{ matrix.plugin.change_type }} |" >> overview-report.md + + # List all Python files for reviewer reference + echo "" >> overview-report.md + echo "
Python files" >> overview-report.md + echo "" >> overview-report.md + find "$src" -name "*.py" -not -path "*/.git/*" | sort | while read f; do + lines=$(wc -l < "$f") + echo "- \`$f\` ($lines lines)" >> overview-report.md + done + echo "" >> overview-report.md + echo "
" >> overview-report.md + + # ── Upload all reports ── + - name: Combine reports + id: report + run: | + { + echo "## 🔐 Security Scan: \`${{ matrix.plugin.name }}\`" + echo "" + echo "**Repository:** ${{ matrix.plugin.repo }}" + echo "**Commit:** \`${{ matrix.plugin.commit }}\`" + echo "**Change type:** ${{ matrix.plugin.change_type }}" + echo "" + echo "---" + echo "" + cat overview-report.md + echo "" + echo "---" + echo "" + cat bandit-report.md + echo "" + echo "---" + echo "" + cat patterns-report.md + echo "" + echo "---" + echo "" + cat semgrep-report.md + echo "" + echo "---" + echo "" + cat deps-report.md + } > full-report.md + + # Determine overall risk level + bandit_fail="${{ steps.bandit.outcome == 'failure' && 'true' || 'false' }}" + critical="${{ steps.patterns.outputs.critical }}" + vuln_count="${{ steps.deps.outputs.vuln_count }}" + + if [ "$critical" = "true" ] || [ "$bandit_fail" = "true" ]; then + echo "risk=HIGH" >> "$GITHUB_OUTPUT" + elif [ "${vuln_count:-0}" -gt 0 ] || [ "${{ steps.patterns.outputs.review }}" = "true" ]; then + echo "risk=MEDIUM" >> "$GITHUB_OUTPUT" + else + echo "risk=LOW" >> "$GITHUB_OUTPUT" + fi + + - name: Upload scan report + uses: actions/upload-artifact@bbbca2ddaa5d8feaa63e36b76fdaad77386f024f # v7 + with: + name: security-report-${{ matrix.plugin.name }} + path: | + full-report.md + bandit-results.json + semgrep-results.json + + # ──────────────────────────────────────────── + # Phase 3: Post combined results to the PR + # ──────────────────────────────────────────── + post-results: + needs: [detect-changes, scan-plugin] + if: always() && needs.detect-changes.outputs.count != '0' + runs-on: ubuntu-latest + steps: + - name: Download all scan reports + uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8 + with: + pattern: security-report-* + + - name: Post combined PR comment + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: | + { + echo "# 🔐 Plugin Security Scan Results" + echo "" + echo "Scanned **${{ needs.detect-changes.outputs.count }}** plugin(s)." + echo "" + + # Concatenate all reports + for dir in security-report-*/; do + if [ -f "${dir}full-report.md" ]; then + cat "${dir}full-report.md" + echo "" + echo "---" + echo "" + fi + done + + echo "" + echo "> **Note:** Automated scans supplement but do not replace manual code review." + echo "> Please examine the flagged items before approving this PR." + } > combined-comment.md + + # Truncate if too long for a PR comment (65536 char limit) + if [ $(wc -c < combined-comment.md) -gt 60000 ]; then + head -c 59000 combined-comment.md > truncated.md + echo "" >> truncated.md + echo "*(Report truncated — see workflow artifacts for full details)*" >> truncated.md + mv truncated.md combined-comment.md + fi + + gh pr comment ${{ needs.detect-changes.outputs.pr_number }} \ + --repo "${{ github.repository }}" \ + --body-file combined-comment.md diff --git a/.github/workflows/validate-pr.yml b/.github/workflows/validate-pr.yml index cb842c4..0c24668 100644 --- a/.github/workflows/validate-pr.yml +++ b/.github/workflows/validate-pr.yml @@ -1,6 +1,7 @@ # .github/workflows/validate-pr.yml # # Runs on every PR that touches repositories.toml. +# Can also be triggered manually by commenting "/validate" on a PR. # Ensures: # 1. The TOML is valid and well-structured # 2. Only one plugin is added or modified per PR @@ -13,6 +14,8 @@ on: pull_request: paths: - "repositories.toml" + issue_comment: + types: [created] permissions: contents: read @@ -21,17 +24,43 @@ permissions: jobs: validate: runs-on: ubuntu-latest + # Run on PR events, or on "/validate" comments on PRs + if: >- + github.event_name == 'pull_request' || + (github.event_name == 'issue_comment' && + github.event.issue.pull_request && + contains(github.event.comment.body, '/validate')) steps: + - name: Get PR details + id: pr + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: | + if [ "${{ github.event_name }}" = "pull_request" ]; then + echo "number=${{ github.event.pull_request.number }}" >> "$GITHUB_OUTPUT" + echo "head_sha=${{ github.event.pull_request.head.sha }}" >> "$GITHUB_OUTPUT" + echo "base_sha=${{ github.event.pull_request.base.sha }}" >> "$GITHUB_OUTPUT" + else + pr_number="${{ github.event.issue.number }}" + echo "number=$pr_number" >> "$GITHUB_OUTPUT" + # Fetch PR refs via the GitHub API + pr_data=$(gh api repos/${{ github.repository }}/pulls/$pr_number) + head_sha=$(echo "$pr_data" | jq -r '.head.sha') + base_sha=$(echo "$pr_data" | jq -r '.base.sha') + echo "head_sha=$head_sha" >> "$GITHUB_OUTPUT" + echo "base_sha=$base_sha" >> "$GITHUB_OUTPUT" + fi + - name: Checkout PR head uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 with: - ref: ${{ github.event.pull_request.head.sha }} + ref: ${{ steps.pr.outputs.head_sha }} path: head - name: Checkout base branch uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 with: - ref: ${{ github.event.pull_request.base.sha }} + ref: ${{ steps.pr.outputs.base_sha }} path: base - uses: actions/setup-python@a309ff8b426b58ec0e2a45f0f869d46889d02405 # v6.2.0 @@ -142,7 +171,7 @@ jobs: See the [workflow summary](${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}) for details." - gh pr comment ${{ github.event.pull_request.number }} --body "$comment" + gh pr comment ${{ steps.pr.outputs.number }} --body "$comment" # ── Final: fail the job if any check failed ── - name: Fail on validation errors From e58b81d5b091c0094217b296cce4e079c8267744 Mon Sep 17 00:00:00 2001 From: Geoff Hutchison Date: Thu, 26 Mar 2026 00:09:01 -0400 Subject: [PATCH 2/4] Some cleanups, including moving bits to a helper script Signed-off-by: Geoff Hutchison --- .github/workflows/security-scan.yml | 366 +++++++++++++++++----------- .github/workflows/validate-pr.yml | 86 +++++-- scripts/security_scan_helpers.py | 283 +++++++++++++++++++++ 3 files changed, 568 insertions(+), 167 deletions(-) create mode 100644 scripts/security_scan_helpers.py diff --git a/.github/workflows/security-scan.yml b/.github/workflows/security-scan.yml index ccdfff5..f6de305 100644 --- a/.github/workflows/security-scan.yml +++ b/.github/workflows/security-scan.yml @@ -21,7 +21,6 @@ on: permissions: contents: read - pull-requests: write jobs: # ──────────────────────────────────────────── @@ -29,16 +28,21 @@ jobs: # ──────────────────────────────────────────── detect-changes: runs-on: ubuntu-latest + permissions: + contents: read + pull-requests: read # Run on PR events, or on "/security" comments on PRs if: >- github.event_name == 'pull_request' || (github.event_name == 'issue_comment' && github.event.issue.pull_request && - contains(github.event.comment.body, '/security')) + startsWith(github.event.comment.body, '/security') && + contains(fromJson('["OWNER", "MEMBER", "COLLABORATOR"]'), github.event.comment.author_association)) outputs: plugins: ${{ steps.diff.outputs.plugins }} count: ${{ steps.diff.outputs.count }} pr_number: ${{ steps.pr.outputs.number }} + base_sha: ${{ steps.pr.outputs.base_sha }} steps: - name: Get PR details id: pr @@ -64,12 +68,14 @@ jobs: with: ref: ${{ steps.pr.outputs.head_sha }} path: head + persist-credentials: false - name: Checkout base branch uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 with: ref: ${{ steps.pr.outputs.base_sha }} path: base + persist-credentials: false - uses: actions/setup-python@a309ff8b426b58ec0e2a45f0f869d46889d02405 # v6.2.0 with: @@ -78,7 +84,7 @@ jobs: - name: Identify changed plugins id: diff run: | - diff_result=$(python head/scripts/parse_plugins.py diff \ + diff_result=$(python3 base/scripts/parse_plugins.py diff \ base/repositories.toml head/repositories.toml) # Build a matrix of plugins that need scanning (added + modified) @@ -119,95 +125,122 @@ jobs: needs: detect-changes if: needs.detect-changes.outputs.count != '0' runs-on: ubuntu-latest + permissions: + contents: read strategy: fail-fast: false matrix: plugin: ${{ fromJson(needs.detect-changes.outputs.plugins) }} steps: - - name: Checkout this repo (for custom rules) + - name: Checkout trusted repo contents uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 with: + ref: ${{ needs.detect-changes.outputs.base_sha }} path: _this + persist-credentials: false - name: Clone plugin at pinned commit - id: clone run: | git clone "${{ matrix.plugin.repo }}" plugin-src cd plugin-src git checkout "${{ matrix.plugin.commit }}" - # Determine the actual source directory - src_path="${{ matrix.plugin.path }}" - if [ "$src_path" != "." ] && [ -d "$src_path" ]; then - echo "src_dir=plugin-src/$src_path" >> "$GITHUB_OUTPUT" - else - echo "src_dir=plugin-src" >> "$GITHUB_OUTPUT" - fi - - uses: actions/setup-python@a309ff8b426b58ec0e2a45f0f869d46889d02405 # v6.2.0 with: python-version: "3.12" + - name: Resolve scan directory + id: src + run: | + repo_root=$(python3 -c "import os; print(os.path.realpath('plugin-src'))") + candidate=$(python3 -c "import os, sys; print(os.path.realpath(os.path.join('plugin-src', sys.argv[1])))" "${{ matrix.plugin.path }}") + + case "$candidate" in + "$repo_root"|"$repo_root"/*) + if [ ! -d "$candidate" ]; then + echo "::error::Configured plugin path does not exist: ${{ matrix.plugin.path }}" + exit 1 + fi + echo "src_dir=$candidate" >> "$GITHUB_OUTPUT" + ;; + *) + echo "::error::Configured plugin path escapes the cloned repository: ${{ matrix.plugin.path }}" + exit 1 + ;; + esac + - name: Install scanning tools run: | - pip install bandit pip-audit safety semgrep + python3 -m pip install --disable-pip-version-check bandit pip-audit semgrep # ── Scan 1: Bandit (Python security linter) ── - name: Run Bandit id: bandit continue-on-error: true run: | - src="${{ steps.clone.outputs.src_dir }}" + src="${{ steps.src.outputs.src_dir }}" echo "### Bandit Results" > bandit-report.md - # Run bandit, capture output + set +e bandit -r "$src" \ -f json \ -ll \ --exclude '**/test*,**/.git' \ - -o bandit-results.json 2>/dev/null || true + -o bandit-results.json 2>/dev/null + bandit_status=$? + set -e + + if [ ! -f bandit-results.json ]; then + echo '{"results":[]}' > bandit-results.json + fi - # Parse results - python3 -c " + BANDIT_STATUS="$bandit_status" python3 - <<'PY' import json - with open('bandit-results.json') as f: + import os + + with open("bandit-results.json", encoding="utf-8") as f: data = json.load(f) - results = data.get('results', []) - high = [r for r in results if r['issue_severity'] == 'HIGH'] - medium = [r for r in results if r['issue_severity'] == 'MEDIUM'] - low = [r for r in results if r['issue_severity'] == 'LOW'] + results = data.get("results", []) + high = [r for r in results if r.get("issue_severity") == "HIGH"] + medium = [r for r in results if r.get("issue_severity") == "MEDIUM"] + low = [r for r in results if r.get("issue_severity") == "LOW"] - with open('bandit-report.md', 'w') as f: - f.write('### 🔍 Bandit (Python Security Linter)\n\n') + with open("bandit-report.md", "w", encoding="utf-8") as f: + f.write("### 🔍 Bandit (Python Security Linter)\n\n") if not results: - f.write('✅ No issues found.\n') + f.write("✅ No issues found.\n") else: - f.write(f'| Severity | Count |\n') - f.write(f'|----------|-------|\n') - f.write(f'| 🔴 High | {len(high)} |\n') - f.write(f'| 🟡 Medium | {len(medium)} |\n') - f.write(f'| 🟢 Low | {len(low)} |\n\n') + f.write("| Severity | Count |\n") + f.write("|----------|-------|\n") + f.write(f"| 🔴 High | {len(high)} |\n") + f.write(f"| 🟡 Medium | {len(medium)} |\n") + f.write(f"| 🟢 Low | {len(low)} |\n\n") if high or medium: - f.write('
Details (high/medium)\n\n') - for r in high + medium: - f.write(f\"\"\"- **{r['issue_severity']}**: {r['issue_text']} - - File: \`{r['filename']}:{r['line_number']}\` - - CWE: {r.get('issue_cwe', {}).get('id', 'N/A')} - - Test: {r['test_id']} - \"\"\") - f.write('\n
\n') - - # Set exit code - exit(1 if high else 0) - " || echo "bandit_failed=true" >> "$GITHUB_OUTPUT" + f.write("
Details (high/medium)\n\n") + for result in high + medium: + f.write( + f"- **{result['issue_severity']}**: {result['issue_text']}\n" + f" - File: `{result['filename']}:{result['line_number']}`\n" + f" - CWE: {result.get('issue_cwe', {}).get('id', 'N/A')}\n" + f" - Test: {result['test_id']}\n" + ) + f.write("\n
\n") + + with open(os.environ["GITHUB_OUTPUT"], "a", encoding="utf-8") as out: + out.write(f"high_count={len(high)}\n") + tool_error = os.environ["BANDIT_STATUS"] not in {"0", "1"} + out.write(f"tool_error={'true' if tool_error else 'false'}\n") + + raise SystemExit(1 if high or tool_error else 0) + PY # ── Scan 2: Dangerous pattern grep ── - name: Check for dangerous patterns id: patterns run: | - src="${{ steps.clone.outputs.src_dir }}" + src="${{ steps.src.outputs.src_dir }}" echo "### 🔎 Dangerous Pattern Check" > patterns-report.md found_critical=false @@ -293,158 +326,183 @@ jobs: id: semgrep continue-on-error: true run: | - src="${{ steps.clone.outputs.src_dir }}" + src="${{ steps.src.outputs.src_dir }}" echo "### 🛡️ Semgrep Analysis" > semgrep-report.md - # Run with community Python security rules + set +e semgrep scan \ --config "p/python" \ --config "p/security-audit" \ --config "p/secrets" \ --json \ --output semgrep-results.json \ - "$src" 2>/dev/null || true + "$src" 2>/dev/null + semgrep_status=$? - # Also run custom Avogadro rules if they exist if [ -f "_this/.github/avogadro-security.yaml" ]; then semgrep scan \ --config "_this/.github/avogadro-security.yaml" \ --json \ --output semgrep-custom-results.json \ - "$src" 2>/dev/null || true + "$src" 2>/dev/null + custom_status=$? + else + custom_status=0 fi + set -e - python3 -c " - import json, os + if [ ! -f semgrep-results.json ]; then + echo '{"results":[]}' > semgrep-results.json + fi + SEMGREP_STATUS="$semgrep_status" CUSTOM_STATUS="$custom_status" python3 - <<'PY' + import json + import os + + severity_order = {"NONE": 0, "INFO": 1, "WARNING": 2, "ERROR": 3} all_results = [] - for fname in ['semgrep-results.json', 'semgrep-custom-results.json']: - if os.path.exists(fname): - with open(fname) as f: - data = json.load(f) - all_results.extend(data.get('results', [])) - - with open('semgrep-report.md', 'w') as f: - f.write('### 🛡️ Semgrep Analysis\n\n') + max_severity = "NONE" + + for fname in ["semgrep-results.json", "semgrep-custom-results.json"]: + if not os.path.exists(fname): + continue + with open(fname, encoding="utf-8") as f: + data = json.load(f) + for result in data.get("results", []): + severity = result.get("extra", {}).get("severity", "INFO") + all_results.append(result) + if severity_order.get(severity, 0) > severity_order[max_severity]: + max_severity = severity + + with open("semgrep-report.md", "w", encoding="utf-8") as f: + f.write("### 🛡️ Semgrep Analysis\n\n") if not all_results: - f.write('✅ No issues found.\n') + f.write("✅ No issues found.\n") else: - # Group by severity by_severity = {} - for r in all_results: - sev = r.get('extra', {}).get('severity', 'INFO') - by_severity.setdefault(sev, []).append(r) - - f.write(f'Found **{len(all_results)}** issue(s):\n\n') - f.write('| Severity | Count |\n') - f.write('|----------|-------|\n') - for sev in ['ERROR', 'WARNING', 'INFO']: + for result in all_results: + sev = result.get("extra", {}).get("severity", "INFO") + by_severity.setdefault(sev, []).append(result) + + f.write(f"Found **{len(all_results)}** issue(s):\n\n") + f.write("| Severity | Count |\n") + f.write("|----------|-------|\n") + for sev in ["ERROR", "WARNING", "INFO"]: items = by_severity.get(sev, []) if items: - icon = {'ERROR': '🔴', 'WARNING': '🟡', 'INFO': 'ℹ️'}.get(sev, '⚪') - f.write(f'| {icon} {sev} | {len(items)} |\n') - - f.write('\n
Issue details\n\n') - for r in all_results: - sev = r.get('extra', {}).get('severity', 'INFO') - msg = r.get('extra', {}).get('message', 'No message') - path = r.get('path', '?') - line = r.get('start', {}).get('line', '?') - rule = r.get('check_id', '?') - f.write(f'- **[{sev}]** \`{path}:{line}\` — {msg} ({rule})\n') - f.write('\n
\n') - " + icon = {"ERROR": "🔴", "WARNING": "🟡", "INFO": "ℹ️"}.get(sev, "⚪") + f.write(f"| {icon} {sev} | {len(items)} |\n") + + f.write("\n
Issue details\n\n") + for result in all_results: + sev = result.get("extra", {}).get("severity", "INFO") + msg = result.get("extra", {}).get("message", "No message") + path = result.get("path", "?") + line = result.get("start", {}).get("line", "?") + rule = result.get("check_id", "?") + f.write(f"- **[{sev}]** `{path}:{line}` - {msg} ({rule})\n") + f.write("\n
\n") + + tool_error = "true" if os.environ["SEMGREP_STATUS"] not in {"0", "1"} or os.environ["CUSTOM_STATUS"] not in {"0", "1"} else "false" + with open(os.environ["GITHUB_OUTPUT"], "a", encoding="utf-8") as out: + out.write(f"max_severity={max_severity}\n") + out.write(f"tool_error={tool_error}\n") + PY # ── Scan 4: Dependency audit ── - name: Audit dependencies id: deps continue-on-error: true run: | - src="${{ steps.clone.outputs.src_dir }}" + src="${{ steps.src.outputs.src_dir }}" echo "### 📦 Dependency Audit" > deps-report.md - # Look for dependency declarations - has_deps=false - dep_file="" - - if [ -f "$src/pyproject.toml" ]; then - dep_file="$src/pyproject.toml" - has_deps=true - elif [ -f "$src/requirements.txt" ]; then - dep_file="$src/requirements.txt" - has_deps=true - elif [ -f "$src/setup.py" ]; then - dep_file="$src/setup.py" - has_deps=true - fi + python3 _this/scripts/security_scan_helpers.py extract-deps "$src" extracted-requirements.txt > deps-summary.json - if [ "$has_deps" = "false" ]; then - echo "ℹ️ No dependency file found (pyproject.toml, requirements.txt, setup.py)." >> deps-report.md + python3 - <<'PY' + import json + + with open("deps-summary.json", encoding="utf-8") as f: + summary = json.load(f) + + with open("deps-report.md", "a", encoding="utf-8") as report: + files = summary.get("dependency_files", []) + if files: + report.write("Dependency files:\n") + for path in files: + report.write(f"- `{path}`\n") + report.write("\n") + else: + report.write("ℹ️ No dependency file found (pyproject.toml, requirements.txt, setup.py).\n") + + if summary.get("dynamic_sources"): + report.write("#### Manual Review Needed\n") + for item in summary["dynamic_sources"]: + report.write(f"- `{item['file']}`: {item['reason']}\n") + report.write("\n") + + if summary.get("non_pypi"): + report.write("#### Dependency Source Check\n") + report.write("⚠️ **Non-PyPI dependency sources found** (review carefully):\n") + for item in summary["non_pypi"]: + report.write(f"- `{item['source']}`: `{item['dependency']}` ({item['reason']})\n") + report.write("\n") + else: + report.write("#### Dependency Source Check\n") + report.write("✅ No non-PyPI dependency sources detected in static dependency files.\n\n") + PY + + if [ ! -s extracted-requirements.txt ]; then + echo "#### pip-audit" >> deps-report.md + echo "ℹ️ No statically auditable dependencies found." >> deps-report.md echo "vuln_count=0" >> "$GITHUB_OUTPUT" exit 0 fi - echo "Dependency file: \`$dep_file\`" >> deps-report.md - echo "" >> deps-report.md - - # pip-audit echo "#### pip-audit" >> deps-report.md - if [ -f "$src/requirements.txt" ]; then - pip-audit -r "$src/requirements.txt" --format json --output pip-audit.json 2>/dev/null || true - elif [ -f "$src/pyproject.toml" ]; then - # Create a temporary venv and install to audit - python -m venv /tmp/audit-venv - source /tmp/audit-venv/bin/activate - pip install "$src" 2>/dev/null || pip install -e "$src" 2>/dev/null || true - pip-audit --format json --output pip-audit.json 2>/dev/null || true - deactivate - fi + set +e + pip-audit -r extracted-requirements.txt --format json --output pip-audit.json 2>/dev/null + audit_status=$? + set -e if [ -f "pip-audit.json" ]; then - python3 -c " + python3 - <<'PY' > /tmp/vuln_count import json - with open('pip-audit.json') as f: + + with open("pip-audit.json", encoding="utf-8") as f: data = json.load(f) - deps = data if isinstance(data, list) else data.get('dependencies', []) - vulns = [d for d in deps if d.get('vulns')] + deps = data if isinstance(data, list) else data.get("dependencies", []) + vulns = [dep for dep in deps if dep.get("vulns")] - with open('deps-report.md', 'a') as f: + with open("deps-report.md", "a", encoding="utf-8") as report: if not vulns: - f.write('✅ No known vulnerabilities in dependencies.\n') + report.write("✅ No known vulnerabilities in dependencies.\n") else: - f.write(f'⚠️ **{len(vulns)} package(s) with known vulnerabilities:**\n\n') - for d in vulns: - f.write(f\"\"\"- **{d['name']}** {d.get('version', '?')}:\n\"\"\") - for v in d['vulns']: - f.write(f\"\"\" - {v.get('id', '?')}: {v.get('description', 'N/A')[:100]}\n\"\"\") + report.write(f"⚠️ **{len(vulns)} package(s) with known vulnerabilities:**\n\n") + for dep in vulns: + report.write(f"- **{dep['name']}** {dep.get('version', '?')}:\n") + for vuln in dep["vulns"]: + description = vuln.get("description", "N/A").replace("\n", " ")[:100] + report.write(f" - {vuln.get('id', '?')}: {description}\n") print(len(vulns)) - " > /tmp/vuln_count + PY echo "vuln_count=$(cat /tmp/vuln_count)" >> "$GITHUB_OUTPUT" else echo "⚠️ Could not run pip-audit." >> deps-report.md echo "vuln_count=0" >> "$GITHUB_OUTPUT" fi - - # Check for non-PyPI dependency sources - echo "" >> deps-report.md - echo "#### Dependency Source Check" >> deps-report.md - suspicious=$(grep -rn --include="*.toml" --include="*.txt" --include="*.cfg" \ - -E "(git\+|https?://(?!pypi)|file://)" "$src" 2>/dev/null | head -10 || true) - if [ -n "$suspicious" ]; then - echo "⚠️ **Non-PyPI dependency sources found** (review carefully):" >> deps-report.md - echo '```' >> deps-report.md - echo "$suspicious" >> deps-report.md - echo '```' >> deps-report.md + if [ "$audit_status" -ne 0 ] && [ "$audit_status" -ne 1 ]; then + echo "audit_error=true" >> "$GITHUB_OUTPUT" else - echo "✅ All dependencies appear to use standard PyPI sources." >> deps-report.md + echo "audit_error=false" >> "$GITHUB_OUTPUT" fi # ── Scan 5: Code metrics & overview ── - name: Code overview run: | - src="${{ steps.clone.outputs.src_dir }}" + src="${{ steps.src.outputs.src_dir }}" echo "### 📊 Code Overview" > overview-report.md # Count Python files and lines @@ -505,10 +563,13 @@ jobs: bandit_fail="${{ steps.bandit.outcome == 'failure' && 'true' || 'false' }}" critical="${{ steps.patterns.outputs.critical }}" vuln_count="${{ steps.deps.outputs.vuln_count }}" + semgrep_max="${{ steps.semgrep.outputs.max_severity }}" + semgrep_error="${{ steps.semgrep.outputs.tool_error }}" + audit_error="${{ steps.deps.outputs.audit_error }}" - if [ "$critical" = "true" ] || [ "$bandit_fail" = "true" ]; then + if [ "$critical" = "true" ] || [ "$bandit_fail" = "true" ] || [ "$semgrep_max" = "ERROR" ]; then echo "risk=HIGH" >> "$GITHUB_OUTPUT" - elif [ "${vuln_count:-0}" -gt 0 ] || [ "${{ steps.patterns.outputs.review }}" = "true" ]; then + elif [ "${vuln_count:-0}" -gt 0 ] || [ "${{ steps.patterns.outputs.review }}" = "true" ] || [ "$semgrep_max" = "WARNING" ] || [ "$semgrep_error" = "true" ] || [ "$audit_error" = "true" ]; then echo "risk=MEDIUM" >> "$GITHUB_OUTPUT" else echo "risk=LOW" >> "$GITHUB_OUTPUT" @@ -518,6 +579,7 @@ jobs: uses: actions/upload-artifact@bbbca2ddaa5d8feaa63e36b76fdaad77386f024f # v7 with: name: security-report-${{ matrix.plugin.name }} + if-no-files-found: warn path: | full-report.md bandit-results.json @@ -530,6 +592,9 @@ jobs: needs: [detect-changes, scan-plugin] if: always() && needs.detect-changes.outputs.count != '0' runs-on: ubuntu-latest + permissions: + contents: read + pull-requests: write steps: - name: Download all scan reports uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8 @@ -541,6 +606,8 @@ jobs: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} run: | { + echo "" + echo "" echo "# 🔐 Plugin Security Scan Results" echo "" echo "Scanned **${{ needs.detect-changes.outputs.count }}** plugin(s)." @@ -569,6 +636,17 @@ jobs: mv truncated.md combined-comment.md fi - gh pr comment ${{ needs.detect-changes.outputs.pr_number }} \ - --repo "${{ github.repository }}" \ - --body-file combined-comment.md + comment_id=$(gh api repos/${{ github.repository }}/issues/${{ needs.detect-changes.outputs.pr_number }}/comments \ + --paginate \ + --jq '.[] | select(.user.login == "github-actions[bot]" and (.body | contains(""))) | .id' \ + | tail -n 1) + + if [ -n "$comment_id" ]; then + body=$(cat combined-comment.md) + gh api --method PATCH repos/${{ github.repository }}/issues/comments/"$comment_id" \ + --raw-field body="$body" >/dev/null + else + gh pr comment ${{ needs.detect-changes.outputs.pr_number }} \ + --repo "${{ github.repository }}" \ + --body-file combined-comment.md + fi diff --git a/.github/workflows/validate-pr.yml b/.github/workflows/validate-pr.yml index 0c24668..c97d899 100644 --- a/.github/workflows/validate-pr.yml +++ b/.github/workflows/validate-pr.yml @@ -19,17 +19,21 @@ on: permissions: contents: read - pull-requests: write jobs: validate: runs-on: ubuntu-latest + permissions: + contents: read + pull-requests: write # Run on PR events, or on "/validate" comments on PRs if: >- github.event_name == 'pull_request' || (github.event_name == 'issue_comment' && github.event.issue.pull_request && - contains(github.event.comment.body, '/validate')) + startsWith(github.event.comment.body, '/validate') && + contains(fromJson('["OWNER", "MEMBER", "COLLABORATOR"]'), github.event.comment.author_association)) + steps: - name: Get PR details id: pr @@ -56,25 +60,30 @@ jobs: with: ref: ${{ steps.pr.outputs.head_sha }} path: head + persist-credentials: false - name: Checkout base branch uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 with: ref: ${{ steps.pr.outputs.base_sha }} path: base + persist-credentials: false - uses: actions/setup-python@a309ff8b426b58ec0e2a45f0f869d46889d02405 # v6.2.0 with: python-version: "3.12" - - name: Copy helper scripts - run: cp head/scripts/parse_plugins.py head/scripts/plugin_validation.py . + - name: Prepare trusted helper scripts + run: | + mkdir trusted + cp base/scripts/parse_plugins.py base/scripts/plugin_validation.py base/scripts/generate_index.py trusted/ + cp head/repositories.toml trusted/repositories.toml # ── Step 1: Diff to find what changed ── - name: Detect plugin changes id: diff run: | - diff_result=$(python parse_plugins.py diff base/repositories.toml head/repositories.toml) + diff_result=$(python3 trusted/parse_plugins.py diff base/repositories.toml head/repositories.toml) echo "$diff_result" | python3 -c " import json, sys, os d = json.load(sys.stdin) @@ -82,7 +91,7 @@ jobs: modified = len(d['modified']) removed = len(d['removed']) total = d['total_changes'] - changed = ' '.join(p['name'] for p in d['added'] + d['modified']) + changed = [p['name'] for p in d['added'] + d['modified']] with open(os.environ['GITHUB_STEP_SUMMARY'], 'a') as summary: summary.write('### Plugin Changes\n') @@ -96,25 +105,41 @@ jobs: out.write(f'added={added}\n') out.write(f'modified={modified}\n') out.write(f'removed={removed}\n') - out.write(f'changed={changed}\n') + out.write(f'changed_json={json.dumps(changed, separators=(\",\", \":\"))}\n') " echo "diff_json=$(echo "$diff_result" | jq -c .)" >> "$GITHUB_OUTPUT" # ── Step 2: Validate changed plugin metadata ── - name: Install dependencies - run: pip install PyGitHub + run: python3 -m pip install --disable-pip-version-check "PyGitHub==2.1.1" - name: Validate plugin metadata id: validate env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + CHANGED_JSON: ${{ steps.diff.outputs.changed_json }} run: | echo "### Plugin Validation" >> "$GITHUB_STEP_SUMMARY" - changed="${{ steps.diff.outputs.changed }}" - if [ -z "$changed" ]; then + if [ "$CHANGED_JSON" = "[]" ]; then echo "✅ No added/modified plugins to validate." >> "$GITHUB_STEP_SUMMARY" echo "toml_valid=true" >> "$GITHUB_OUTPUT" - elif python head/scripts/generate_index.py -t "$GH_TOKEN" --strict -p $changed; then + exit 0 + fi + + changed_plugins=() + while IFS= read -r plugin_name; do + changed_plugins+=("$plugin_name") + done < <( + python3 - <<'PY' + import json + import os + + for name in json.loads(os.environ["CHANGED_JSON"]): + print(name) + PY + ) + + if python3 trusted/generate_index.py -t "$GH_TOKEN" --strict -p "${changed_plugins[@]}"; then echo "✅ All changed plugins validated successfully." >> "$GITHUB_STEP_SUMMARY" echo "toml_valid=true" >> "$GITHUB_OUTPUT" else @@ -160,18 +185,33 @@ jobs: status="❌ Some validation checks failed" fi - comment="## Plugin PR Validation - - ${status} - - | Check | Result | - |-------|--------| - | TOML structure | $([ "$toml_ok" = "true" ] && echo "✅ Valid" || echo "❌ Invalid") | - | Single plugin rule | $([ "$single_ok" = "true" ] && echo "✅ Pass ($total change)" || echo "❌ Fail ($total changes)") | - - See the [workflow summary](${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}) for details." - - gh pr comment ${{ steps.pr.outputs.number }} --body "$comment" + { + echo "" + echo "" + echo "## Plugin PR Validation" + echo "" + echo "${status}" + echo "" + echo "| Check | Result |" + echo "|-------|--------|" + echo "| TOML structure | $([ "$toml_ok" = "true" ] && echo "✅ Valid" || echo "❌ Invalid") |" + echo "| Single plugin rule | $([ "$single_ok" = "true" ] && echo "✅ Pass ($total change)" || echo "❌ Fail ($total changes)") |" + echo "" + echo "See the [workflow summary](${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}) for details." + } > comment.md + + comment_id=$(gh api repos/${{ github.repository }}/issues/${{ steps.pr.outputs.number }}/comments \ + --paginate \ + --jq '.[] | select(.user.login == "github-actions[bot]" and (.body | contains(""))) | .id' \ + | tail -n 1) + + if [ -n "$comment_id" ]; then + body=$(cat comment.md) + gh api --method PATCH repos/${{ github.repository }}/issues/comments/"$comment_id" \ + --raw-field body="$body" >/dev/null + else + gh pr comment ${{ steps.pr.outputs.number }} --body-file comment.md + fi # ── Final: fail the job if any check failed ── - name: Fail on validation errors diff --git a/scripts/security_scan_helpers.py b/scripts/security_scan_helpers.py new file mode 100644 index 0000000..1e7515b --- /dev/null +++ b/scripts/security_scan_helpers.py @@ -0,0 +1,283 @@ +#!/usr/bin/env python3 +"""Trusted helpers for the security scan workflow.""" + +from __future__ import annotations + +import argparse +import json +from pathlib import Path +import re +try: + import tomllib +except ModuleNotFoundError: # pragma: no cover - fallback for older local Python + import tomli as tomllib + + +DIRECT_URL_RE = re.compile(r"(?:^|\s)(?:-e\s+)?(?:git\+|https?://|file://)|\s@\s(?:git\+|https?://|file://)") + + +def _append_requirement(requirements: list[str], value: str | None) -> None: + if not value: + return + + normalized = value.strip() + if not normalized: + return + + requirements.append(normalized) + + +def _load_toml(path: Path) -> dict: + with path.open("rb") as f: + return tomllib.load(f) + + +def _is_non_pypi(value: str) -> bool: + return bool(DIRECT_URL_RE.search(value)) + + +def _format_poetry_dependency(name: str, spec) -> tuple[str | None, str | None]: + if name == "python": + return None, None + + if isinstance(spec, str): + requirement = name if spec in {"", "*"} else f"{name}{spec}" + reason = "direct URL dependency" if _is_non_pypi(spec) else None + return requirement, reason + + if not isinstance(spec, dict): + return None, "unsupported dependency format" + + if "git" in spec: + return None, "git dependency" + if "path" in spec: + return None, "path dependency" + if "url" in spec: + return None, "url dependency" + if "source" in spec: + return None, f"custom source {spec['source']}" + + extras = "" + if spec.get("extras"): + extras = "[" + ",".join(spec["extras"]) + "]" + + version = spec.get("version", "") + requirement = f"{name}{extras}" + if version not in {"", "*"}: + requirement += version + if spec.get("markers"): + requirement += f"; {spec['markers']}" + return requirement, None + + +def _collect_requirements_file( + path: Path, + source_label: str, + requirements: list[str], + non_pypi: list[dict[str, str]], + visited: set[Path], +) -> None: + resolved = path.resolve() + if resolved in visited or not path.exists(): + return + + visited.add(resolved) + for raw_line in path.read_text(encoding="utf-8").splitlines(): + line = raw_line.strip() + if not line or line.startswith("#"): + continue + if " #" in line: + line = line.split(" #", 1)[0].strip() + if not line: + continue + + if line.startswith(("-r ", "--requirement ")): + _, include_target = line.split(maxsplit=1) + _collect_requirements_file( + path.parent / include_target, + str(path.parent / include_target), + requirements, + non_pypi, + visited, + ) + continue + + if line.startswith(("-c ", "--constraint ", "--index-url", "--extra-index-url", "--find-links")): + non_pypi.append( + { + "source": source_label, + "dependency": line, + "reason": "custom package index or finder directive", + } + ) + continue + + if _is_non_pypi(line): + non_pypi.append( + { + "source": source_label, + "dependency": line, + "reason": "direct URL dependency", + } + ) + continue + + _append_requirement(requirements, line) + + +def extract_dependencies(src_dir: Path, requirements_out: Path) -> dict: + dependency_files: list[str] = [] + dynamic_sources: list[dict[str, str]] = [] + non_pypi: list[dict[str, str]] = [] + requirements: list[str] = [] + visited: set[Path] = set() + + requirements_txt = src_dir / "requirements.txt" + if requirements_txt.exists(): + dependency_files.append(str(requirements_txt)) + _collect_requirements_file( + requirements_txt, + str(requirements_txt), + requirements, + non_pypi, + visited, + ) + + pyproject = src_dir / "pyproject.toml" + if pyproject.exists(): + dependency_files.append(str(pyproject)) + data = _load_toml(pyproject) + + project = data.get("project", {}) + for dep in project.get("dependencies", []): + if _is_non_pypi(dep): + non_pypi.append( + { + "source": str(pyproject), + "dependency": dep, + "reason": "direct URL dependency", + } + ) + else: + _append_requirement(requirements, dep) + + for group_name, deps in project.get("optional-dependencies", {}).items(): + for dep in deps: + if _is_non_pypi(dep): + non_pypi.append( + { + "source": str(pyproject), + "dependency": dep, + "reason": f"direct URL dependency in optional group {group_name}", + } + ) + else: + _append_requirement(requirements, dep) + + dynamic_fields = set(project.get("dynamic", [])) + if "dependencies" in dynamic_fields or "optional-dependencies" in dynamic_fields: + dynamic_sources.append( + { + "file": str(pyproject), + "reason": "project dependencies are declared dynamically and were not executed", + } + ) + + setuptools_dynamic = data.get("tool", {}).get("setuptools", {}).get("dynamic", {}) + if "dependencies" in setuptools_dynamic or "optional-dependencies" in setuptools_dynamic: + dynamic_sources.append( + { + "file": str(pyproject), + "reason": "setuptools dynamic dependencies were not executed", + } + ) + + poetry = data.get("tool", {}).get("poetry", {}) + poetry_dependencies = poetry.get("dependencies", {}) + for name, spec in poetry_dependencies.items(): + requirement, reason = _format_poetry_dependency(name, spec) + if reason: + non_pypi.append( + { + "source": str(pyproject), + "dependency": name, + "reason": reason, + } + ) + else: + _append_requirement(requirements, requirement) + + poetry_groups = poetry.get("group", {}) + for group_name, group_data in poetry_groups.items(): + for name, spec in group_data.get("dependencies", {}).items(): + requirement, reason = _format_poetry_dependency(name, spec) + if reason: + non_pypi.append( + { + "source": str(pyproject), + "dependency": name, + "reason": f"{reason} in Poetry group {group_name}", + } + ) + else: + _append_requirement(requirements, requirement) + + for group_name, deps in data.get("dependency-groups", {}).items(): + for dep in deps: + if _is_non_pypi(dep): + non_pypi.append( + { + "source": str(pyproject), + "dependency": dep, + "reason": f"direct URL dependency in dependency group {group_name}", + } + ) + else: + _append_requirement(requirements, dep) + + setup_py = src_dir / "setup.py" + if setup_py.exists(): + dependency_files.append(str(setup_py)) + dynamic_sources.append( + { + "file": str(setup_py), + "reason": "setup.py dependencies were not executed", + } + ) + + deduped_requirements = list(dict.fromkeys(requirements)) + requirements_out.write_text( + "".join(f"{requirement}\n" for requirement in deduped_requirements), + encoding="utf-8", + ) + + return { + "dependency_files": dependency_files, + "dynamic_sources": dynamic_sources, + "non_pypi": non_pypi, + "requirements": deduped_requirements, + } + + +def main() -> None: + parser = argparse.ArgumentParser(description="Trusted helpers for security scans.") + subparsers = parser.add_subparsers(dest="command", required=True) + + extract_parser = subparsers.add_parser( + "extract-deps", + help="Extract statically auditable dependency requirements.", + ) + extract_parser.add_argument("src_dir", help="Plugin source directory") + extract_parser.add_argument("requirements_out", help="Output requirements file path") + + args = parser.parse_args() + + if args.command == "extract-deps": + src_dir = Path(args.src_dir) + requirements_out = Path(args.requirements_out) + result = extract_dependencies(src_dir, requirements_out) + print(json.dumps(result, indent=2)) + + +if __name__ == "__main__": + main() From db80da6843488d6b193cb8f61400436cb6d220ab Mon Sep 17 00:00:00 2001 From: Geoff Hutchison Date: Thu, 26 Mar 2026 00:15:41 -0400 Subject: [PATCH 3/4] Based on code review, make sure to only update one entry Limit permissions to particular steps Be careful about manual workflow-dispatch names Signed-off-by: Geoff Hutchison --- .github/workflows/check-plugin-updates.yml | 59 ++++------- scripts/parse_plugins.py | 13 ++- scripts/update_plugin_entry.py | 109 +++++++++++++++++++++ 3 files changed, 140 insertions(+), 41 deletions(-) create mode 100644 scripts/update_plugin_entry.py diff --git a/.github/workflows/check-plugin-updates.yml b/.github/workflows/check-plugin-updates.yml index 453b4e6..c7b8151 100644 --- a/.github/workflows/check-plugin-updates.yml +++ b/.github/workflows/check-plugin-updates.yml @@ -18,17 +18,20 @@ on: type: string permissions: - contents: write - pull-requests: write + contents: read jobs: check-updates: runs-on: ubuntu-latest + permissions: + contents: read outputs: updates: ${{ steps.detect.outputs.updates }} count: ${{ steps.detect.outputs.count }} steps: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + with: + persist-credentials: false - uses: actions/setup-python@a309ff8b426b58ec0e2a45f0f869d46889d02405 # v6.2.0 with: @@ -36,19 +39,15 @@ jobs: - name: Detect upstream updates id: detect + env: + PLUGIN_NAME: ${{ inputs.plugin_name }} run: | - updates=$(python scripts/parse_plugins.py check-updates) - echo "Raw updates: $updates" - - # If a specific plugin was requested, filter to just that one - if [ -n "${{ inputs.plugin_name }}" ]; then - updates=$(echo "$updates" | python3 -c " - import json, sys - data = json.load(sys.stdin) - filtered = [p for p in data if p['name'] == '${{ inputs.plugin_name }}'] - print(json.dumps(filtered)) - ") + if [ -n "$PLUGIN_NAME" ]; then + updates=$(python3 scripts/parse_plugins.py check-updates --plugin-name "$PLUGIN_NAME") + else + updates=$(python3 scripts/parse_plugins.py check-updates) fi + echo "Raw updates: $updates" count=$(echo "$updates" | python3 -c "import json,sys; print(len(json.load(sys.stdin)))") echo "updates=$(echo "$updates" | jq -c .)" >> "$GITHUB_OUTPUT" @@ -60,6 +59,9 @@ jobs: needs: check-updates if: needs.check-updates.outputs.count != '0' runs-on: ubuntu-latest + permissions: + contents: write + pull-requests: write strategy: # Process one plugin at a time to avoid branch conflicts max-parallel: 1 @@ -125,31 +127,12 @@ jobs: new_commit="${{ matrix.plugin.latest_commit }}" latest_tag="${{ steps.commit-info.outputs.latest_tag }}" - # Replace the commit SHA - sed -i "s|$old_commit|$new_commit|" repositories.toml - - # If we found a release tag, update or add release-tag - if [ -n "$latest_tag" ]; then - # Check if release-tag already exists for this plugin - if grep -A5 "^\[${plugin}\]" repositories.toml | grep -q "release-tag"; then - # Update existing release-tag (within the plugin's section) - python3 -c " - import re - with open('repositories.toml', 'r') as f: - content = f.read() - # Find the plugin section and update release-tag within it - pattern = r'(\[${plugin}\].*?release-tag\s*=\s*)\"[^\"]*\"' - content = re.sub(pattern, r'\1\"${latest_tag}\"', content, flags=re.DOTALL) - with open('repositories.toml', 'w') as f: - f.write(content) - " - else - # Add release-tag after the commit line - sed -i "/^\[${plugin}\]/,/^$\|^\[/ { - /git\.commit/a release-tag = \"${latest_tag}\" - }" repositories.toml - fi - fi + python3 scripts/update_plugin_entry.py \ + repositories.toml \ + "$plugin" \ + "$old_commit" \ + "$new_commit" \ + --latest-tag "$latest_tag" - name: Create Pull Request if: steps.check-pr.outputs.skip == 'false' diff --git a/scripts/parse_plugins.py b/scripts/parse_plugins.py index 32089ed..d72b8d1 100644 --- a/scripts/parse_plugins.py +++ b/scripts/parse_plugins.py @@ -38,7 +38,7 @@ def cmd_list(): print(json.dumps(result, indent=2)) -def cmd_check_updates(): +def cmd_check_updates(plugin_name: str | None = None): """ For each git-based plugin, check if the upstream default branch has moved past the pinned commit. Output JSON with update info. @@ -48,6 +48,9 @@ def cmd_check_updates(): updates = [] for name, info in plugins.items(): + if plugin_name and name != plugin_name: + continue + git = info.get("git", {}) repo_url = git.get("repo", "") pinned_commit = git.get("commit", "") @@ -162,10 +165,14 @@ def main(): ) subparsers = parser.add_subparsers(dest="command", required=True) - subparsers.add_parser( + check_updates_parser = subparsers.add_parser( "check-updates", help="Check for upstream updates (new commits on default branch)", ) + check_updates_parser.add_argument( + "--plugin-name", + help="Only check the named plugin", + ) diff_parser = subparsers.add_parser( "diff", help="Diff two versions of repositories.toml to find changes" @@ -176,7 +183,7 @@ def main(): args = parser.parse_args() if args.command == "check-updates": - cmd_check_updates() + cmd_check_updates(args.plugin_name) elif args.command == "diff": cmd_diff(args.base_file, args.head_file) diff --git a/scripts/update_plugin_entry.py b/scripts/update_plugin_entry.py new file mode 100644 index 0000000..5f2010c --- /dev/null +++ b/scripts/update_plugin_entry.py @@ -0,0 +1,109 @@ +#!/usr/bin/env python3 +"""Update a single plugin section in repositories.toml.""" + +from __future__ import annotations + +import argparse +import re +from pathlib import Path + + +def _find_section_bounds(lines: list[str], plugin_name: str) -> tuple[int, int]: + header = f"[{plugin_name}]" + start = None + + for idx, line in enumerate(lines): + if line.strip() == header: + start = idx + break + + if start is None: + raise ValueError(f"Plugin section [{plugin_name}] not found") + + end = len(lines) + for idx in range(start + 1, len(lines)): + stripped = lines[idx].strip() + if stripped.startswith("[") and stripped.endswith("]"): + end = idx + break + + return start, end + + +def update_plugin_entry( + file_path: Path, + plugin_name: str, + old_commit: str, + new_commit: str, + latest_tag: str | None, +) -> None: + lines = file_path.read_text(encoding="utf-8").splitlines(keepends=True) + start, end = _find_section_bounds(lines, plugin_name) + section = lines[start:end] + + commit_pattern = re.compile(r'^(\s*git\.commit\s*=\s*")([0-9a-f]{40})(".*)$') + commit_index = None + + for idx, line in enumerate(section): + match = commit_pattern.match(line) + if not match: + continue + if match.group(2) != old_commit: + raise ValueError( + f"Plugin [{plugin_name}] commit did not match expected value {old_commit}" + ) + section[idx] = f'{match.group(1)}{new_commit}{match.group(3)}\n' + commit_index = idx + break + + if commit_index is None: + raise ValueError(f"Plugin [{plugin_name}] does not contain git.commit") + + release_pattern = re.compile(r'^(\s*release-tag\s*=\s*")([^"]*)(".*)$') + release_index = None + for idx, line in enumerate(section): + if release_pattern.match(line): + release_index = idx + break + + if latest_tag: + new_release_line = f'release-tag = "{latest_tag}"\n' + if release_index is not None: + section[release_index] = new_release_line + else: + insert_at = commit_index + 1 + if insert_at < len(section) and section[insert_at].strip(): + section.insert(insert_at, new_release_line) + else: + section.insert(insert_at, new_release_line) + elif release_index is not None: + del section[release_index] + + lines[start:end] = section + file_path.write_text("".join(lines), encoding="utf-8") + + +def main() -> None: + parser = argparse.ArgumentParser(description="Update a single plugin entry.") + parser.add_argument("file_path", help="Path to repositories.toml") + parser.add_argument("plugin_name", help="Plugin table name") + parser.add_argument("old_commit", help="Expected current commit SHA") + parser.add_argument("new_commit", help="New commit SHA") + parser.add_argument( + "--latest-tag", + default="", + help="Optional release tag to set; omit or pass empty string to remove it", + ) + args = parser.parse_args() + + update_plugin_entry( + Path(args.file_path), + args.plugin_name, + args.old_commit, + args.new_commit, + args.latest_tag or None, + ) + + +if __name__ == "__main__": + main() From 0570a6837fd7ab8b13880248bbc4d2931c72225f Mon Sep 17 00:00:00 2001 From: Geoff Hutchison Date: Thu, 26 Mar 2026 14:17:45 -0400 Subject: [PATCH 4/4] Apply suggestions from code review Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> --- .github/avogadro-security.yaml | 5 ++++- .github/workflows/validate-pr.yml | 4 +++- scripts/security_scan_helpers.py | 8 ++++++++ 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/.github/avogadro-security.yaml b/.github/avogadro-security.yaml index df46041..7881537 100644 --- a/.github/avogadro-security.yaml +++ b/.github/avogadro-security.yaml @@ -33,7 +33,10 @@ rules: - pattern: _pickle.load(...) - pattern: marshal.load(...) - pattern: marshal.loads(...) - - pattern: yaml.load(...) # without Loader=SafeLoader + - patterns: + - pattern: yaml.load(...) + - pattern-not: yaml.load(..., Loader=yaml.SafeLoader, ...) + - pattern-not: yaml.load(..., Loader=SafeLoader, ...) - pattern: shelve.open(...) message: > Unsafe deserialization can execute arbitrary code. Use JSON or diff --git a/.github/workflows/validate-pr.yml b/.github/workflows/validate-pr.yml index c97d899..a3dcdf1 100644 --- a/.github/workflows/validate-pr.yml +++ b/.github/workflows/validate-pr.yml @@ -210,7 +210,9 @@ jobs: gh api --method PATCH repos/${{ github.repository }}/issues/comments/"$comment_id" \ --raw-field body="$body" >/dev/null else - gh pr comment ${{ steps.pr.outputs.number }} --body-file comment.md + gh pr comment ${{ steps.pr.outputs.number }} \ + --repo "${{ github.repository }}" \ + --body-file comment.md fi # ── Final: fail the job if any check failed ── diff --git a/scripts/security_scan_helpers.py b/scripts/security_scan_helpers.py index 1e7515b..2e9ffd4 100644 --- a/scripts/security_scan_helpers.py +++ b/scripts/security_scan_helpers.py @@ -224,6 +224,14 @@ def extract_dependencies(src_dir: Path, requirements_out: Path) -> dict: for group_name, deps in data.get("dependency-groups", {}).items(): for dep in deps: + if isinstance(dep, dict): + dynamic_sources.append( + { + "file": str(pyproject), + "reason": f"dependency group {group_name} contains a non-string item and was not expanded", + } + ) + continue if _is_non_pypi(dep): non_pypi.append( {