diff --git a/.github/instructions/github_issue_workflow.instructions.md b/.github/instructions/github_issue_workflow.instructions.md new file mode 100644 index 000000000..897dac360 --- /dev/null +++ b/.github/instructions/github_issue_workflow.instructions.md @@ -0,0 +1,52 @@ +--- +applyTo: '**' +--- + +# GitHub Issue Workflow + +## When Starting Work + +At the start of meaningful work, determine whether there is an associated GitHub issue for the task. + +- If the user mentions an issue number or URL, treat that as the associated issue and keep it in mind for summaries, PR notes, release notes, and later status updates. +- If no issue is mentioned and the work is more than a tiny clarification or one-off command, ask the admin whether they want to create an issue before or alongside the work. +- If the admin wants an issue created, use the existing `Create GitHub Issue` prompt in `.github/prompts/create-github-issue.prompt.md`. +- Do not block urgent fixes, small investigations, or explicitly time-sensitive work just because there is no issue yet. Continue the work and ask at the next natural checkpoint. + +## Issue Association + +When an issue exists or is created for the work: + +- Track it as the working associated issue for the current task. +- Reference the issue in relevant summaries, PR descriptions, fix documentation, feature documentation, and release note entries when appropriate. +- Prefer durable references such as `Fixes #123`, `Closes #123`, or `Refs #123` when the change directly resolves, completes, or relates to the issue. +- Do not create duplicate issues. Use the issue creation prompt's duplicate-search workflow before creating a new issue. + +## When To Suggest Updating Issues + +Prompt the admin to update the associated issue at natural checkpoints, not on every interaction. + +Ask whether to update the associated issue when any of these occur: + +- The implementation is complete or materially changes direction. +- Important validation results are available. +- Scope, priority, acceptance criteria, or user impact changes. +- A blocker, dependency, risk, or follow-up task is discovered. +- Release notes are being updated or the admin is asked whether to update release notes. + +Use concise wording such as: + +> Would you like me to update the associated GitHub issue with the summary, validation, and any follow-ups from this work? + +If there is no associated issue at one of these checkpoints, ask whether the admin wants to create one using the `Create GitHub Issue` prompt. + +## What To Put In Issue Updates + +When the admin approves an issue update, include only useful status information: + +- What changed. +- Current validation status and relevant test results. +- Any unresolved risks, blockers, or follow-ups. +- Links or references to related PRs, documentation, or release notes when available. + +Do not post noisy progress comments, implementation chatter, or repeated updates that do not change the issue's useful state. \ No newline at end of file diff --git a/.github/instructions/update_release_notes.instructions.md b/.github/instructions/update_release_notes.instructions.md index 353cea48b..f6828eeee 100644 --- a/.github/instructions/update_release_notes.instructions.md +++ b/.github/instructions/update_release_notes.instructions.md @@ -10,6 +10,8 @@ After completing a code change (bug fix, new feature, enhancement, or breaking c **"Would you like me to update the release notes in `docs/explanation/release_notes.md`?"** +When asking this, also ask whether the associated GitHub issue should be updated. If there is no associated issue, ask whether the admin wants to create one using `.github/prompts/create-github-issue.prompt.md`. + ## If the User Confirms Yes Update the release notes file following these guidelines: diff --git a/.github/prompts/malicious-pr-and-file-security-review.prompt.md b/.github/prompts/malicious-pr-and-file-security-review.prompt.md new file mode 100644 index 000000000..861d1a0ba --- /dev/null +++ b/.github/prompts/malicious-pr-and-file-security-review.prompt.md @@ -0,0 +1,282 @@ +--- +description: "Use when: reviewing a SimpleChat pull request, commit diff, branch diff, or explicit file set for malicious code, hidden security issues, obfuscation, unauthorized external connections, malware-like behavior, data exfiltration, dependency pinning, or newly released package risk." +name: "Malicious PR And File Security Review" +argument-hint: "PR number/URL, base/head refs, commit range, explicit file paths, scan only, dependency gate only, or fix findings" +agent: "agent" +--- + +# Malicious PR And File Security Review + +Review a SimpleChat pull request, commit range, branch diff, or explicit file set for intentionally harmful behavior, hidden security issues, supply-chain risk, obfuscation, external network egress, data exfiltration, dependency policy violations, and contributor trickery. + +This prompt is for adversarial review. Assume benign explanations are possible, but do not give suspicious code the benefit of the doubt without evidence. The default output is a findings report, not edits. Only modify files if the user explicitly asks to remediate findings. + +## Scope Inputs + +The user may provide any of these targets: + +- A GitHub PR number or URL. +- Base and head refs, such as `upstream/Development...feature-branch`. +- A commit range. +- A list of explicit files. +- A pasted diff. + +If the target is ambiguous, use the safest local interpretation and state it. If no target is provided, review the current working tree and branch diff against the repository's normal PR target, `Development`. + +## Operating Rules + +- Work in the SimpleChat repository root. +- Treat this as a security review with hostile-change assumptions. +- Do not revert unrelated user changes. +- Do not run untrusted code, install new packages, execute changed scripts, start services, or run package lifecycle hooks from the reviewed change unless the user explicitly approves after seeing the risk. +- Prefer read-only inspection, static search, metadata review, and existing repo checkers. +- If registry, GitHub, or network access is unavailable, report which checks could not be completed and whether that blocks approval. +- If the user asks for `scan only`, `review only`, or `report only`, do not edit files. +- If findings are remediated, keep fixes focused and rerun the relevant checks. + +## Baseline Discovery + +1. Confirm repository, branch, status, and target scope: + +```powershell +git rev-parse --show-toplevel +git branch --show-current +git status --short +git remote -v +``` + +2. If reviewing a PR and GitHub CLI is available, capture PR metadata and files: + +```powershell +gh pr view --json number,title,author,baseRefName,headRefName,commits,files,additions,deletions,reviews,statusCheckRollup +gh pr diff --name-only +gh pr diff +``` + +3. If reviewing local refs, identify changed files and patch content: + +```powershell +git fetch --all --prune +git diff --name-status ... +git diff --stat ... +git diff --find-renames --find-copies ... +``` + +4. If reviewing explicit files, inspect each file and, when possible, compare it to the base branch: + +```powershell +git diff -- +git diff ...HEAD -- +``` + +5. Classify changed files before deep review: + +- Application runtime: `application/single_app/**`. +- Browser runtime: templates, static JavaScript, CSS, workers, WASM, fonts, vendored assets. +- Routes/auth/security: Flask routes, decorators, role checks, CSRF, CSP, settings sanitization. +- Dependency manifests: `requirements*.txt`, `package.json`, lockfiles, Dockerfiles, GitHub Actions, installer scripts. +- Infrastructure/deployment: `deployers/**`, Bicep, Terraform, PowerShell, Azure CLI scripts, `azd` hooks. +- Tests and validation: functional tests, UI tests, route policy tests, security scanners, CI workflows. +- Documentation or prompts that may influence operational behavior. + +## Dependency Gate + +Apply this policy to new or changed dependencies in any ecosystem, and especially to Python requirements files: + +- Dependencies must be exactly pinned. Reject ranges such as `>=`, `>`, `<`, `<=`, `~=`, `^`, `~`, `*`, npm dist-tags such as `latest`, unversioned entries, and floating Docker tags unless the repository already has a reviewed exception. +- Python direct dependencies in `requirements*.txt` must use `package==version`. +- Reject editable installs, local paths, direct URLs, Git URLs, untrusted package indexes, `--extra-index-url`, and `--trusted-host` unless explicitly justified and reviewed. +- Reject any new or changed package version released less than seven full days before the review date. +- If the release date cannot be verified from a trusted registry or lockfile metadata, mark the dependency as `Needs investigation` or `Blocker`, depending on whether it is production-reachable. +- Treat pre-release, yanked, typosquatting-suspect, abandoned, newly transferred, or dependency-confusion-risk packages as suspicious even when older than seven days. + +Find changed dependency manifests: + +```powershell +git diff --name-only ... | Select-String -Pattern "requirements.*\.txt$|package\.json$|package-lock\.json$|npm-shrinkwrap\.json$|pnpm-lock\.yaml$|yarn\.lock$|pyproject\.toml$|poetry\.lock$|Pipfile$|Pipfile\.lock$|Dockerfile$|\.github/workflows/|deployers/" +``` + +Find unsafe Python requirement forms: + +```powershell +rg -n "^\s*(-e\s+|--extra-index-url|--index-url|--trusted-host|https?://|git\+|file:|[A-Za-z0-9_.-]+\s*(>=|>|<=|<|~=|!=)|[A-Za-z0-9_.-]+\s*$)" -g "requirements*.txt" +``` + +Verify Python package release dates through PyPI when network access is available: + +```powershell +$reviewDate = Get-Date +$package = "" +$version = "" +$metadata = Invoke-RestMethod "https://pypi.org/pypi/$package/$version/json" +$uploads = $metadata.urls | Select-Object -ExpandProperty upload_time_iso_8601 +$oldestUpload = ($uploads | ForEach-Object { [datetime]$_ } | Sort-Object | Select-Object -First 1) +$age = $reviewDate - $oldestUpload +$age.TotalDays +``` + +Verify npm package release dates when JavaScript dependencies changed: + +```powershell +npm view @ time --json +``` + +For Docker images, GitHub Actions, Terraform providers, Bicep modules, PowerShell modules, browser libraries, and other dependency ecosystems, verify exact pinning and release age from the relevant trusted registry when possible. Prefer immutable digests, commit SHAs, or exact versions over tags. + +## High-Risk Search Patterns + +Run targeted searches over changed files first, then broaden if suspicious patterns appear. + +### External Connections And Egress + +Look for new or changed code that connects to public or unexpected endpoints, sends telemetry, calls webhooks, opens sockets, uses DNS as a channel, or loads remote assets: + +```powershell +rg -n -i "https?://|webhook|telemetry|analytics|beacon|sendBeacon|fetch\(|XMLHttpRequest|WebSocket|EventSource|requests\.|httpx\.|aiohttp|urllib|socket\.|smtplib|ftplib|paramiko|scp|Invoke-WebRequest|Invoke-RestMethod|curl|wget|nslookup|Resolve-DnsName|cdn\.|unpkg|jsdelivr|cdnjs|esm\.sh|skypack" +``` + +Review whether the endpoint is expected, configurable, documented, local-only where required, and not receiving secrets, prompts, files, embeddings, tokens, cookies, settings, logs, or user data. + +### Secret, Credential, And Data Exfiltration Sources + +Look for code that reads sensitive values and pairs them with network, logging, serialization, or process execution sinks: + +```powershell +rg -n -i "api[_-]?key|secret|password|passwd|pwd|token|bearer|credential|connection[_ -]?string|client[_-]?secret|private[_-]?key|account[_-]?key|sas[_-]?token|Authorization|Cookie|get_settings\(|os\.environ|process\.env|localStorage|sessionStorage|document\.cookie|id_rsa|\.env|MSAL|OPENAI|COSMOS|SEARCH|BLOB|STORAGE|KEYVAULT|GRAPH" +``` + +Review logs, errors, exports, traces, activity records, plugin calls, prompt construction, browser storage, hidden inputs, generated artifacts, and test fixtures for accidental or intentional disclosure. + +### Obfuscation, Stealth, And Hidden Payloads + +Look for code designed to hide intent or evade review: + +```powershell +rg -n -i "base64|b64decode|atob\(|fromCharCode|charCodeAt|unescape\(|decodeURIComponent|zlib|gzip|brotli|marshal|pickle|loads\(|exec\(|eval\(|compile\(|new Function|Function\(|importlib|__import__|getattr\(|setattr\(|globals\(\)|locals\(\)|Reflection|Add-Type|EncodedCommand|FromBase64String|IEX|Invoke-Expression|Start-Process|hidden|homoglyph|bidi|unicode" +``` + +Also inspect: + +- Very long strings, high-entropy blobs, minified code, generated code, binary artifacts, unexpected images/documents, source maps, compressed archives, and vendored files. +- Unicode bidirectional controls, zero-width characters, homoglyph identifiers, trailing code after comments, misleading filenames, and whitespace-only changes. +- Renames, copied files, or large deletions that may hide a small malicious change. + +Useful checks: + +```powershell +git diff --check ... +git diff --word-diff=color ... -- +rg -n --pcre2 "[\x{202A}-\x{202E}\x{2066}-\x{2069}\x{200B}\x{200C}\x{200D}\x{FEFF}]" +``` + +### Dynamic Execution, Persistence, And System Access + +Look for code that executes commands, changes the host, installs software, persists tasks, or reaches local files unexpectedly: + +```powershell +rg -n -i "subprocess|os\.system|popen|shell=True|pty|spawn|execFile|child_process|ProcessStartInfo|Start-Process|New-Service|schtasks|crontab|systemctl|chmod|chown|icacls|reg add|Set-ItemProperty|pip install|npm install|postinstall|preinstall|setup.py|pyproject.toml|entry_points|ctypes|cffi|ffi|DllImport|LoadLibrary|unsafe" +``` + +Do not execute changed lifecycle scripts or installers as part of the review. + +### Security Control Tampering + +Look for changes that weaken existing controls, create blind spots, or make malicious behavior harder to detect: + +```powershell +rg -n -i "login_required|admin_required|user_required|swagger_route|get_auth_security|csrf|Content-Security-Policy|sanitize_settings_for_user|escapeHtml|DOMPurify|innerHTML|outerHTML|insertAdjacentHTML|eval\(|debug\s*=\s*True|verify\s*=\s*False|ssl|cert|validate|allowlist|denylist|permission|role|policy|redact|mask|log_event|audit|telemetry|traceback|try:|except Exception|pass|skip|xfail|disable|noqa|type:\s*ignore|pragma" +``` + +Pay special attention to: + +- Removed route decorators, Blueprint auth policies, role checks, CSRF checks, CSP restrictions, XSS sanitization, settings sanitization, or secret redaction. +- Test changes that reduce coverage, skip security tests, weaken assertions, or alter fixtures to hide failures. +- CI changes that remove scanners, lower permissions boundaries, expose tokens to pull_request contexts, add untrusted actions, or run contributor code with secrets. +- Logging changes that suppress errors, hide audit trails, or send sensitive telemetry externally. + +### AI, Plugin, And Agent-Specific Risks + +Review SimpleChat AI surfaces for prompt, tool, and data exfiltration risks: + +- New tools, plugins, OpenAPI specs, external action endpoints, or agent instructions that can send prompts, chat history, uploaded documents, embeddings, citations, settings, or user identity outside approved services. +- Prompt-injection pathways that bypass workspace isolation or role checks. +- Hidden system prompt changes, model endpoint changes, unreviewed fallback models, or external model providers. +- Changes to document ingestion, search indexing, citations, file sync, public workspaces, or group workspace boundaries that could leak documents across users or workspaces. + +## Manual Review Checklist + +Review each changed file with these questions: + +- What new trust boundary is crossed? +- What data can leave the process, browser, container, tenant, subscription, or repository? +- Does the code read secrets, environment variables, settings, tokens, cookies, prompts, uploaded files, generated files, or private documents? +- Is any new external host, package registry, CDN, model endpoint, webhook, telemetry sink, storage account, or database introduced? +- Are dependencies pinned exactly and older than seven full days? +- Can the change run during install, import, test, build, deploy, startup, request handling, browser load, or CI before a human notices? +- Does the change weaken authentication, authorization, CSRF, CSP, XSS defenses, settings sanitization, redaction, audit logging, route policy tests, or deployment security? +- Is code intentionally hard to read through obfuscation, indirection, hidden Unicode, minification, misleading names, dead code, broad exception swallowing, or noisy unrelated churn? +- Are new binaries, archives, generated assets, vendored libraries, certificates, keys, models, or serialized files justified and inspectable? +- Could this change behave differently in CI, production, Azure, a container, Windows, Linux, or when environment variables are present? + +## Triage + +Group findings by severity: + +- `Critical`: Clear credential exfiltration, malware-like behavior, unauthorized external egress of sensitive data, install-time execution from an untrusted dependency, active security control bypass, CI secret exposure to untrusted code, malicious obfuscation with reachable execution, or a dependency policy violation in production code. +- `Important`: Suspicious external endpoint, newly introduced broad system/process access, weakened auth or sanitization, risky package provenance, unverified package release age, unsafe CI permissions, or hidden behavior that could plausibly expose data. +- `Moderate`: Exact risk depends on configuration or reachability, suspicious but non-production dependency change, stale/binary/opaque vendor asset, broad logging, broad exception swallowing, or weakened tests without a direct exploit path. +- `Low`: Review hygiene concerns, false positives, documentation-only external links, unreachable code, or already-reviewed exceptions. + +For every finding, include: + +- Severity and verdict: `Blocker`, `Needs investigation`, or `Acceptable with notes`. +- File and relevant symbol or changed hunk. +- Evidence from diff, file contents, registry metadata, command output, or repo policy. +- Source data involved and sink or execution path. +- Reachability: install time, import time, build time, CI, browser load, request path, admin-only, test-only, deployer-only, or unreachable. +- Why benign explanations are insufficient or what evidence would clear the concern. +- Recommended action. +- Minimum validation after remediation. + +## Validation + +Use existing repo checks when they match the changed surface. Do not execute untrusted changed code just to validate it. + +Run static or deterministic checks such as: + +```powershell +git -c core.whitespace=blank-at-eol,blank-at-eof,space-before-tab,cr-at-eol diff --check +python scripts/check_xss_sinks.py --full-file +python scripts/check_broken_access_control.py --full-file +python scripts/check_swagger_routes.py +``` + +For changed Python files that are not suspicious lifecycle/install code, run syntax checks: + +```powershell +python -m py_compile +``` + +For changed JavaScript files, run parse checks: + +```powershell +node --check +``` + +If dependencies changed, validate pinning and release age from trusted registries. If package install, build, or lifecycle execution is required to test, stop and ask before running it. + +## Output Expectations + +Return findings first, ordered by severity. Include: + +- Review target and base/head or file scope. +- Changed-file summary grouped by risk area. +- Dependency policy result, including exact pins, release-age checks, package sources, and any unverified packages. +- External hosts, URLs, registries, CDNs, webhooks, model endpoints, or telemetry sinks introduced or modified. +- Obfuscation, hidden Unicode, binary, generated, vendored, or minified artifacts reviewed. +- Security controls weakened, removed, or confirmed unchanged. +- Commands and tools run, with pass/fail/skipped status. +- False positives and reviewed exceptions. +- Final verdict: `Block`, `Needs investigation`, or `No malicious indicators found in reviewed scope`. + +Do not claim the code is safe. Say only what was reviewed, what evidence was found, what could not be checked, and what risk remains. \ No newline at end of file diff --git a/.github/workflows/malicious-pr-security-review.yml b/.github/workflows/malicious-pr-security-review.yml new file mode 100644 index 000000000..414f0dfd0 --- /dev/null +++ b/.github/workflows/malicious-pr-security-review.yml @@ -0,0 +1,147 @@ +name: Malicious PR Security Review + +on: + pull_request: + branches: + - Development + paths: + - 'application/**' + - 'deployers/**' + - 'docker-customization/**' + - 'scripts/**' + - 'functional_tests/**' + - 'ui_tests/**' + - 'requirements*.txt' + - '**/requirements*.txt' + - '**/package.json' + - '**/package-lock.json' + - '**/npm-shrinkwrap.json' + - '**/pnpm-lock.yaml' + - '**/yarn.lock' + - '**/pyproject.toml' + - '**/poetry.lock' + - '**/Pipfile' + - '**/Pipfile.lock' + - '**/Dockerfile' + - '.github/workflows/**' + - '.github/prompts/**' + - '.github/instructions/**' + - 'docs/**' + workflow_dispatch: + inputs: + base_ref: + description: 'Base ref or SHA for the review range' + required: true + default: 'Development' + head_ref: + description: 'Head ref or SHA for the review range. Defaults to the workflow SHA.' + required: false + default: '' + full_file_scan: + description: 'Scan full changed files instead of only changed lines' + type: boolean + required: false + default: false + verify_release_age: + description: 'Query PyPI/npm metadata for the seven-day dependency gate' + type: boolean + required: false + default: true + fail_on_findings: + description: 'Fail on all findings instead of only blockers' + type: boolean + required: false + default: false + fail_on_unverified_release_age: + description: 'Fail when dependency release age cannot be verified' + type: boolean + required: false + default: false + +permissions: + contents: read + pull-requests: read + +jobs: + malicious-pr-security-review: + runs-on: ubuntu-latest + + steps: + - name: Checkout code + uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - name: Set up Python + uses: actions/setup-python@v5 + with: + python-version: '3.12' + + - name: Resolve review range + id: review-range + shell: bash + run: | + if [[ "${{ github.event_name }}" == "pull_request" ]]; then + echo "base_sha=${{ github.event.pull_request.base.sha }}" >> "$GITHUB_OUTPUT" + echo "head_sha=${{ github.sha }}" >> "$GITHUB_OUTPUT" + exit 0 + fi + + git fetch --all --prune + base_ref="${{ inputs.base_ref }}" + head_ref="${{ inputs.head_ref }}" + if [[ -z "$head_ref" ]]; then + head_ref="${{ github.sha }}" + fi + + base_sha="$(git rev-parse "origin/${base_ref}^{commit}" 2>/dev/null || git rev-parse "${base_ref}^{commit}")" + head_sha="$(git rev-parse "${head_ref}^{commit}")" + + echo "base_sha=$base_sha" >> "$GITHUB_OUTPUT" + echo "head_sha=$head_sha" >> "$GITHUB_OUTPUT" + + - name: Run malicious PR static security review + env: + BASE_SHA: ${{ steps.review-range.outputs.base_sha }} + HEAD_SHA: ${{ steps.review-range.outputs.head_sha }} + run: | + review_args=( + --base-sha "$BASE_SHA" + --head-sha "$HEAD_SHA" + --report-file artifacts/malicious-pr-security-review.md + ) + + if [[ "${{ github.event_name }}" == "workflow_dispatch" && "${{ inputs.full_file_scan }}" == "true" ]]; then + review_args+=(--full-file) + fi + + if [[ "${{ github.event_name }}" != "workflow_dispatch" || "${{ inputs.verify_release_age }}" == "true" ]]; then + review_args+=(--verify-release-age) + fi + + if [[ "${{ github.event_name }}" == "workflow_dispatch" && "${{ inputs.fail_on_findings }}" == "true" ]]; then + review_args+=(--fail-on-findings) + fi + + if [[ "${{ github.event_name }}" == "workflow_dispatch" && "${{ inputs.fail_on_unverified_release_age }}" == "true" ]]; then + review_args+=(--fail-on-unverified-release-age) + fi + + python scripts/check_malicious_pr_security_review.py "${review_args[@]}" + + - name: Publish review summary + if: always() + run: | + if [[ -f artifacts/malicious-pr-security-review.md ]]; then + cat artifacts/malicious-pr-security-review.md >> "$GITHUB_STEP_SUMMARY" + else + echo "Malicious PR security review report was not created." >> "$GITHUB_STEP_SUMMARY" + fi + + - name: Upload review report + if: always() + uses: actions/upload-artifact@v4 + with: + name: malicious-pr-security-review + path: artifacts/malicious-pr-security-review.md + if-no-files-found: warn \ No newline at end of file diff --git a/application/single_app/config.py b/application/single_app/config.py index 1ac390f41..d2e0efc4c 100644 --- a/application/single_app/config.py +++ b/application/single_app/config.py @@ -95,7 +95,7 @@ EXECUTOR_TYPE = 'thread' EXECUTOR_MAX_WORKERS = 30 SESSION_TYPE = 'filesystem' -VERSION = "0.250.005" +VERSION = "0.250.006" SESSION_COOKIE_SAMESITE = os.getenv('SESSION_COOKIE_SAMESITE', 'Lax') SESSION_COOKIE_HTTPONLY = os.getenv('SESSION_COOKIE_HTTPONLY', 'true').lower() != 'false' diff --git a/docs/explanation/release_notes.md b/docs/explanation/release_notes.md index f9fd14272..91d7af88d 100644 --- a/docs/explanation/release_notes.md +++ b/docs/explanation/release_notes.md @@ -2,6 +2,15 @@ For feature-focused and fix-focused drill-downs by version, see [Features by Version](/explanation/features/) and [Fixes by Version](/explanation/fixes/). +### **(v0.250.006)** + +#### New Features + +* **Malicious PR Security Review Workflow** + * Added a static malicious-change review workflow for pull requests into `Development`, with manual dispatch options for custom review ranges and full-file scans. + * Added a reusable security review prompt and focused functional coverage for dependency pinning policy, hidden Unicode detection, suspicious egress markers, and workflow wiring. + * (Ref: malicious PR security review, `.github/workflows/malicious-pr-security-review.yml`, `scripts/check_malicious_pr_security_review.py`) + ### **(v0.250.005)** #### New Features diff --git a/functional_tests/test_malicious_pr_security_review_checker.py b/functional_tests/test_malicious_pr_security_review_checker.py new file mode 100644 index 000000000..1bf5776c7 --- /dev/null +++ b/functional_tests/test_malicious_pr_security_review_checker.py @@ -0,0 +1,159 @@ +#!/usr/bin/env python3 +# test_malicious_pr_security_review_checker.py +""" +Functional test for Malicious PR Security Review CI guardrails. +Version: 0.250.006 +Implemented in: 0.250.006 + +This test ensures the malicious PR/file security review checker flags the +highest-risk dependency and hidden-payload patterns, records suspicious egress +markers for human review, and stays wired into the GitHub Actions workflow. +""" + +import importlib.util +import sys +from pathlib import Path + + +ROOT_DIR = Path(__file__).resolve().parents[1] +CHECKER_FILE = ROOT_DIR / 'scripts' / 'check_malicious_pr_security_review.py' +WORKFLOW_FILE = ROOT_DIR / '.github' / 'workflows' / 'malicious-pr-security-review.yml' +PROMPT_FILE = ROOT_DIR / '.github' / 'prompts' / 'malicious-pr-and-file-security-review.prompt.md' +CONFIG_FILE = ROOT_DIR / 'application' / 'single_app' / 'config.py' + + +def read_text(path: Path) -> str: + """Read a UTF-8 text file from the repository.""" + return path.read_text(encoding='utf-8') + + +def load_checker_module(): + """Import the checker module from disk without mutating sys.path.""" + spec = importlib.util.spec_from_file_location('check_malicious_pr_security_review', CHECKER_FILE) + assert spec is not None and spec.loader is not None, 'Expected a module spec for the checker' + module = importlib.util.module_from_spec(spec) + sys.modules[spec.name] = module + spec.loader.exec_module(module) + return module + + +def read_config_version() -> str: + """Extract the current application version from config.py.""" + for line in read_text(CONFIG_FILE).splitlines(): + if line.strip().startswith('VERSION = '): + return line.split('=', 1)[1].strip().strip('"') + raise AssertionError('VERSION assignment not found in config.py') + + +def test_dependency_gate_blocks_unpinned_python_requirements_and_collects_exact_pins() -> None: + """Verify unsafe Python requirement forms are blockers and exact pins are collected.""" + module = load_checker_module() + findings = [] + releases = [] + requirements_source = """ +requests==2.32.4 +flask>=3.0 +--extra-index-url https://packages.example.invalid/simple +git+https://example.invalid/package.git +""".strip() + + module.inspect_requirements_file( + ROOT_DIR / 'requirements-test.txt', + requirements_source, + findings, + releases, + ) + + blocker_messages = [finding.message for finding in findings if finding.verdict == 'Blocker'] + assert any('not exactly pinned' in message for message in blocker_messages), blocker_messages + assert any('custom index' in message for message in blocker_messages), blocker_messages + assert any('direct URL' in message for message in blocker_messages), blocker_messages + assert [(release.package_name, release.version) for release in releases] == [('requests', '2.32.4')] + + +def test_checker_flags_hidden_unicode_and_records_suspicious_egress_markers() -> None: + """Verify hidden Unicode blocks and external egress markers are reported.""" + module = load_checker_module() + findings = [] + source = "fetch('https://example.invalid/webhook', { method: 'POST' });\n" + "\u202e" + + urls = module.scan_text_patterns( + ROOT_DIR / 'application' / 'single_app' / 'static' / 'js' / 'sample.js', + source, + changed_lines=None, + full_file=True, + findings=findings, + ) + + assert 'https://example.invalid/webhook' in urls + assert any(finding.verdict == 'Blocker' and 'Hidden Unicode' in finding.message for finding in findings), findings + assert any('external connection' in finding.message for finding in findings), findings + + +def test_npm_and_docker_dependency_policy_review() -> None: + """Verify npm floating ranges and Docker latest tags are rejected.""" + module = load_checker_module() + findings = [] + releases = [] + + module.inspect_package_json( + ROOT_DIR / 'package.json', + '{"dependencies":{"safe":"1.2.3","floating":"^4.5.6"}}', + findings, + releases, + ) + module.inspect_dockerfile( + ROOT_DIR / 'Dockerfile', + 'FROM python:latest\n', + findings, + ) + + blocker_messages = [finding.message for finding in findings if finding.verdict == 'Blocker'] + assert any('npm dependency is not pinned' in message for message in blocker_messages), blocker_messages + assert any('Docker base image is floating' in message for message in blocker_messages), blocker_messages + assert [(release.package_name, release.version) for release in releases] == [('safe', '1.2.3')] + + +def test_checker_workflow_prompt_and_version_are_wired_into_repo() -> None: + """Verify the checker, workflow, prompt, and version bump landed together.""" + assert CHECKER_FILE.exists(), f'Expected checker script at {CHECKER_FILE}' + assert WORKFLOW_FILE.exists(), f'Expected workflow file at {WORKFLOW_FILE}' + assert PROMPT_FILE.exists(), f'Expected prompt file at {PROMPT_FILE}' + assert read_config_version() == '0.250.006' + + workflow_source = read_text(WORKFLOW_FILE) + assert 'Malicious PR Security Review' in workflow_source + assert 'pull_request' in workflow_source + assert 'workflow_dispatch' in workflow_source + assert 'scripts/check_malicious_pr_security_review.py' in workflow_source + assert '--verify-release-age' in workflow_source + assert 'actions/upload-artifact@v4' in workflow_source + + prompt_source = read_text(PROMPT_FILE) + assert 'Dependency Gate' in prompt_source + assert 'High-Risk Search Patterns' in prompt_source + assert 'Do not run untrusted code' in prompt_source + + +if __name__ == '__main__': + tests = [ + test_dependency_gate_blocks_unpinned_python_requirements_and_collects_exact_pins, + test_checker_flags_hidden_unicode_and_records_suspicious_egress_markers, + test_npm_and_docker_dependency_policy_review, + test_checker_workflow_prompt_and_version_are_wired_into_repo, + ] + results = [] + + for test in tests: + print(f'Running {test.__name__}...') + try: + test() + print('PASS') + results.append(True) + except Exception as exc: + print(f'FAIL: {exc}') + results.append(False) + + success = all(results) + print(f'Results: {sum(results)}/{len(results)} tests passed') + sys.exit(0 if success else 1) \ No newline at end of file diff --git a/scripts/check_malicious_pr_security_review.py b/scripts/check_malicious_pr_security_review.py new file mode 100644 index 000000000..d965262b0 --- /dev/null +++ b/scripts/check_malicious_pr_security_review.py @@ -0,0 +1,894 @@ +# check_malicious_pr_security_review.py + +"""Run static malicious-change review checks for changed files.""" + +from __future__ import annotations + +import argparse +import json +import re +import shutil +import subprocess +import sys +from dataclasses import dataclass +from datetime import datetime, timezone +from pathlib import Path +from urllib.error import HTTPError, URLError +from urllib.request import urlopen + + +REPO_ROOT = Path(__file__).resolve().parents[1] +DIFF_HUNK_RE = re.compile(r'^@@ -\d+(?:,\d+)? \+(?P\d+)(?:,(?P\d+))? @@') +HIDDEN_UNICODE_RE = re.compile('[\u202a-\u202e\u2066-\u2069\u200b\u200c\u200d\ufeff]') +URL_RE = re.compile(r'https?://[^\s\'"<>)]+', re.IGNORECASE) +PYTHON_REQUIREMENT_RE = re.compile( + r'^(?P[A-Za-z0-9_.-]+(?:\[[A-Za-z0-9_,.-]+\])?)==(?P[A-Za-z0-9][A-Za-z0-9_.!+\-]*)' + r'(?:\s*;\s*.+)?$' +) +NPM_EXACT_VERSION_RE = re.compile(r'^\d+\.\d+\.\d+(?:[-+][0-9A-Za-z.-]+)?$') +GITHUB_ACTION_REF_RE = re.compile(r'uses:\s*([^\s#]+@([^\s#]+))') +DOCKER_FROM_RE = re.compile(r'^\s*FROM\s+(?P[^\s]+)', re.IGNORECASE) + +REVIEW_RULES = [ + ( + 'Important', + 'Needs investigation', + 'external connection or remote asset marker', + re.compile( + r'https?://|webhook|telemetry|analytics|beacon|sendBeacon|fetch\(|XMLHttpRequest|WebSocket|EventSource|' + r'requests\.|httpx\.|aiohttp|urllib|socket\.|smtplib|ftplib|paramiko|scp|Invoke-WebRequest|' + r'Invoke-RestMethod|curl|wget|nslookup|Resolve-DnsName|cdn\.|unpkg|jsdelivr|cdnjs|esm\.sh|skypack', + re.IGNORECASE, + ), + 'Review whether changed code can send prompts, files, credentials, cookies, settings, logs, or user data to a new sink.', + 'network, browser-load, deploy, or CI path', + ), + ( + 'Important', + 'Needs investigation', + 'secret or sensitive data source marker', + re.compile( + r'api[_-]?key|secret|password|passwd|pwd|token|bearer|credential|connection[_ -]?string|client[_-]?secret|' + r'private[_-]?key|account[_-]?key|sas[_-]?token|Authorization|Cookie|get_settings\(|os\.environ|process\.env|' + r'localStorage|sessionStorage|document\.cookie|id_rsa|\.env|MSAL|OPENAI|COSMOS|SEARCH|BLOB|STORAGE|KEYVAULT|GRAPH', + re.IGNORECASE, + ), + 'Pair this source with any nearby network, logging, serialization, or process execution sink before approving.', + 'request, browser, import, deploy, or CI path', + ), + ( + 'Moderate', + 'Needs investigation', + 'obfuscation, dynamic loading, or hidden payload marker', + re.compile( + r'base64|b64decode|atob\(|fromCharCode|charCodeAt|unescape\(|decodeURIComponent|zlib|gzip|brotli|' + r'marshal|pickle|loads\(|exec\(|eval\(|compile\(|new Function|Function\(|importlib|__import__|getattr\(|' + r'setattr\(|globals\(\)|locals\(\)|Reflection|Add-Type|EncodedCommand|FromBase64String|IEX|Invoke-Expression|' + r'Start-Process|hidden|homoglyph|bidi|unicode', + re.IGNORECASE, + ), + 'Confirm the changed code is not hiding behavior, decoding payloads, or bypassing normal review.', + 'import, request, browser, deploy, or CI path', + ), + ( + 'Important', + 'Needs investigation', + 'dynamic execution, persistence, or system access marker', + re.compile( + r'subprocess|os\.system|popen|shell=True|pty|spawn|execFile|child_process|ProcessStartInfo|Start-Process|' + r'New-Service|schtasks|crontab|systemctl|chmod|chown|icacls|reg add|Set-ItemProperty|pip install|npm install|' + r'postinstall|preinstall|setup\.py|pyproject\.toml|entry_points|ctypes|cffi|ffi|DllImport|LoadLibrary|unsafe', + re.IGNORECASE, + ), + 'Do not execute changed lifecycle scripts or installers while this finding is unresolved.', + 'install, import, build, deploy, CI, or request path', + ), + ( + 'Important', + 'Needs investigation', + 'security control, sanitization, or audit marker', + re.compile( + r'login_required|admin_required|user_required|swagger_route|get_auth_security|csrf|Content-Security-Policy|' + r'sanitize_settings_for_user|escapeHtml|DOMPurify|innerHTML|outerHTML|insertAdjacentHTML|eval\(|debug\s*=\s*True|' + r'verify\s*=\s*False|ssl|cert|validate|allowlist|denylist|permission|role|policy|redact|mask|log_event|audit|' + r'telemetry|traceback|try:|except Exception|pass|skip|xfail|disable|noqa|type:\s*ignore|pragma', + re.IGNORECASE, + ), + 'Confirm the change does not weaken auth, CSRF, CSP, XSS defenses, settings sanitization, redaction, audit logging, or tests.', + 'request, browser, test, or CI path', + ), + ( + 'Important', + 'Needs investigation', + 'AI, plugin, agent, or workspace boundary marker', + re.compile( + r'kernel_function|OpenAPI|plugin|agent|tool|prompt|instructions|chat_history|conversation|citation|embedding|' + r'public_workspace|group_workspace|workspace_id|model_endpoint|azure_openai|semantic_kernel', + re.IGNORECASE, + ), + 'Check whether prompts, chat history, uploaded documents, embeddings, citations, settings, or identity can cross a new boundary.', + 'agent, plugin, request, search, or model path', + ), +] + +RISK_AREAS = { + 'application runtime': ('application/single_app/', 'application/external_apps/', 'application/teams_app/'), + 'browser runtime': ('.html', '.js', '.css', '.wasm', '.map', '.woff', '.woff2'), + 'dependency manifest': ( + 'requirements', + 'package.json', + 'package-lock.json', + 'npm-shrinkwrap.json', + 'pnpm-lock.yaml', + 'yarn.lock', + 'pyproject.toml', + 'poetry.lock', + 'Pipfile', + 'Pipfile.lock', + 'Dockerfile', + ), + 'infrastructure and deployment': ('deployers/', '.github/workflows/', 'Dockerfile', '.bicep', '.tf', '.ps1', 'azure.yaml'), + 'tests and validation': ('functional_tests/', 'ui_tests/', 'scripts/check_', 'pytest.ini'), + 'documentation and prompts': ('docs/', '.github/prompts/', '.github/instructions/'), +} + + +@dataclass(frozen=True) +class Finding: + """A static review finding.""" + + severity: str + verdict: str + file_path: str + line: int + message: str + evidence: str + reachability: str + recommendation: str + + +@dataclass(frozen=True) +class DependencyRelease: + """A dependency version selected for optional release-age verification.""" + + ecosystem: str + package_name: str + version: str + file_path: str + line: int + + +def get_relative_path(file_path: Path) -> str: + """Return a repository-relative path when possible.""" + try: + return file_path.resolve().relative_to(REPO_ROOT).as_posix() + except ValueError: + return file_path.as_posix() + + +def run_git(args: list[str]) -> subprocess.CompletedProcess[str]: + """Run a git command from the repository root.""" + return subprocess.run( + ['git', *args], + cwd=REPO_ROOT, + check=False, + capture_output=True, + text=True, + encoding='utf-8', + errors='replace', + ) + + +def get_changed_files(base_sha: str | None, head_sha: str | None) -> list[Path]: + """Return changed paths from git.""" + if base_sha and head_sha: + diff_args = ['diff', '--name-only', '--diff-filter=ACMRT', base_sha, head_sha] + else: + diff_args = ['diff', '--name-only', '--diff-filter=ACMRT', 'HEAD'] + + result = run_git(diff_args) + if result.returncode not in {0, 1}: + raise RuntimeError(result.stderr.strip() or 'Unable to collect changed files from git') + + return [REPO_ROOT / path for path in result.stdout.splitlines() if path.strip()] + + +def normalize_paths(raw_paths: list[str]) -> list[Path]: + """Resolve explicit CLI paths relative to the repository root.""" + paths: list[Path] = [] + for raw_path in raw_paths: + candidate = Path(raw_path) + if not candidate.is_absolute(): + candidate = REPO_ROOT / candidate + paths.append(candidate.resolve()) + return paths + + +def get_changed_lines(file_path: Path, base_sha: str | None, head_sha: str | None) -> set[int] | None: + """Return added-line numbers for a file between revisions, or None for full-file mode.""" + if not base_sha or not head_sha: + return None + + relative_path = get_relative_path(file_path) + result = run_git(['diff', '--unified=0', base_sha, head_sha, '--', relative_path]) + if result.returncode not in {0, 1}: + return None + + changed_lines: set[int] = set() + for line in result.stdout.splitlines(): + match = DIFF_HUNK_RE.match(line) + if not match: + continue + start_line = int(match.group('start')) + line_count = int(match.group('count') or '1') + if line_count > 0: + changed_lines.update(range(start_line, start_line + line_count)) + + return changed_lines + + +def is_text_file(file_path: Path) -> bool: + """Return True when a file can be read as UTF-8-ish text.""" + try: + sample = file_path.read_bytes()[:4096] + except OSError: + return False + return b'\x00' not in sample + + +def read_text(file_path: Path) -> str | None: + """Read a text file with replacement for invalid byte sequences.""" + if not file_path.exists() or not file_path.is_file() or not is_text_file(file_path): + return None + return file_path.read_text(encoding='utf-8', errors='replace') + + +def line_is_changed(line_number: int, changed_lines: set[int] | None, full_file: bool) -> bool: + """Return True when a line should be inspected.""" + return full_file or changed_lines is None or line_number in changed_lines + + +def add_finding( + findings: list[Finding], + severity: str, + verdict: str, + file_path: Path, + line: int, + message: str, + evidence: str, + reachability: str, + recommendation: str, +) -> None: + """Append a finding with normalized path and evidence.""" + findings.append( + Finding( + severity=severity, + verdict=verdict, + file_path=get_relative_path(file_path), + line=line, + message=message, + evidence=evidence.strip()[:400], + reachability=reachability, + recommendation=recommendation, + ) + ) + + +def scan_text_patterns( + file_path: Path, + source_text: str, + changed_lines: set[int] | None, + full_file: bool, + findings: list[Finding], +) -> set[str]: + """Scan one text file for suspicious static review markers.""" + discovered_urls: set[str] = set() + for line_number, line in enumerate(source_text.splitlines(), start=1): + if not line_is_changed(line_number, changed_lines, full_file): + continue + + for url_match in URL_RE.finditer(line): + discovered_urls.add(url_match.group(0).rstrip('.,')) + + if HIDDEN_UNICODE_RE.search(line): + add_finding( + findings, + 'Critical', + 'Blocker', + file_path, + line_number, + 'Hidden Unicode control character found in changed content.', + line, + 'review-time and runtime parsing path', + 'Remove the hidden character or replace the line with plain visible text before approval.', + ) + + for severity, verdict, label, pattern, recommendation, reachability in REVIEW_RULES: + if pattern.search(line): + add_finding( + findings, + severity, + verdict, + file_path, + line_number, + f'Changed line contains {label}.', + line, + reachability, + recommendation, + ) + + return discovered_urls + + +def inspect_binary_file(file_path: Path, findings: list[Finding]) -> None: + """Add a review finding for opaque changed binary content.""" + if not file_path.exists() or file_path.is_dir(): + return + add_finding( + findings, + 'Moderate', + 'Needs investigation', + file_path, + 1, + 'Changed file is binary or opaque to text review.', + file_path.name, + 'build, browser, documentation, or deploy artifact path', + 'Confirm the binary, generated asset, archive, certificate, model, or document is expected and inspectable.', + ) + + +def is_requirement_file(relative_path: str) -> bool: + """Return True for Python requirements manifests.""" + name = Path(relative_path).name.lower() + return name.startswith('requirements') and name.endswith('.txt') + + +def is_dependency_manifest(relative_path: str) -> bool: + """Return True when a path should receive dependency-policy review.""" + path = relative_path.lower() + name = Path(path).name + return ( + is_requirement_file(relative_path) + or name in {'package.json', 'package-lock.json', 'npm-shrinkwrap.json', 'pnpm-lock.yaml', 'yarn.lock'} + or name in {'pyproject.toml', 'poetry.lock', 'pipfile', 'pipfile.lock', 'dockerfile'} + or path.endswith('/dockerfile') + or path.startswith('.github/workflows/') + ) + + +def inspect_requirements_file( + file_path: Path, + source_text: str, + findings: list[Finding], + releases: list[DependencyRelease], +) -> None: + """Validate exact Python requirements pins and collect versions for release-age checks.""" + for line_number, raw_line in enumerate(source_text.splitlines(), start=1): + stripped = raw_line.strip() + if not stripped or stripped.startswith('#') or stripped.startswith('--hash='): + continue + + if stripped.startswith(('-e ', '--extra-index-url', '--index-url', '--trusted-host')): + add_finding( + findings, + 'Critical', + 'Blocker', + file_path, + line_number, + 'Python requirement uses an editable install, custom index, or trusted-host override.', + raw_line, + 'dependency resolution or install time', + 'Use reviewed public registry packages pinned with package==version, or document and approve an exception.', + ) + continue + + if re.search(r'https?://|git\+|file:', stripped, re.IGNORECASE): + add_finding( + findings, + 'Critical', + 'Blocker', + file_path, + line_number, + 'Python requirement uses a direct URL, Git URL, or local file source.', + raw_line, + 'dependency resolution or install time', + 'Replace with a reviewed package==version pin from the approved registry.', + ) + continue + + if stripped.startswith(('-r ', '--requirement', '-c ', '--constraint')): + add_finding( + findings, + 'Moderate', + 'Needs investigation', + file_path, + line_number, + 'Python requirement delegates dependency resolution to another file.', + raw_line, + 'dependency resolution or install time', + 'Review the referenced file in the same gate and ensure all direct dependencies are exactly pinned.', + ) + continue + + match = PYTHON_REQUIREMENT_RE.match(stripped) + if not match: + add_finding( + findings, + 'Critical', + 'Blocker', + file_path, + line_number, + 'Python requirement is not exactly pinned with package==version.', + raw_line, + 'dependency resolution or install time', + 'Use exact package==version pins. Reject ranges, exclusions, wildcards, and unversioned entries.', + ) + continue + + package_name = match.group('name').split('[', 1)[0] + version = match.group('version') + if '*' in version: + add_finding( + findings, + 'Critical', + 'Blocker', + file_path, + line_number, + 'Python requirement version contains a wildcard.', + raw_line, + 'dependency resolution or install time', + 'Use an exact package==version pin without wildcards.', + ) + if re.search(r'(a|b|rc|dev)\d*', version, re.IGNORECASE): + add_finding( + findings, + 'Important', + 'Needs investigation', + file_path, + line_number, + 'Python requirement appears to use a pre-release version.', + raw_line, + 'dependency resolution or install time', + 'Confirm the pre-release package was intentionally selected and reviewed.', + ) + releases.append(DependencyRelease('pypi', package_name, version, get_relative_path(file_path), line_number)) + + +def inspect_package_json( + file_path: Path, + source_text: str, + findings: list[Finding], + releases: list[DependencyRelease], +) -> None: + """Validate exact npm package pins and collect versions for release-age checks.""" + try: + package_data = json.loads(source_text) + except json.JSONDecodeError as exc: + add_finding( + findings, + 'Critical', + 'Blocker', + file_path, + exc.lineno, + 'package.json could not be parsed as JSON.', + exc.msg, + 'dependency resolution or install time', + 'Fix JSON syntax before dependency review can complete.', + ) + return + + sections = ('dependencies', 'devDependencies', 'optionalDependencies', 'peerDependencies') + for section in sections: + dependencies = package_data.get(section, {}) + if not isinstance(dependencies, dict): + continue + for package_name, specifier in sorted(dependencies.items()): + evidence = f'{section}.{package_name}: {specifier}' + if not isinstance(specifier, str) or not NPM_EXACT_VERSION_RE.match(specifier): + add_finding( + findings, + 'Critical', + 'Blocker', + file_path, + 1, + 'npm dependency is not pinned to an exact version.', + evidence, + 'dependency resolution or install time', + 'Use exact semver versions. Reject ranges, dist-tags, workspace/file/Git/URL dependencies, and wildcards.', + ) + continue + if re.search(r'-(?:alpha|beta|rc|next|dev)', specifier, re.IGNORECASE): + add_finding( + findings, + 'Important', + 'Needs investigation', + file_path, + 1, + 'npm dependency appears to use a pre-release version.', + evidence, + 'dependency resolution or install time', + 'Confirm the pre-release package was intentionally selected and reviewed.', + ) + releases.append(DependencyRelease('npm', package_name, specifier, get_relative_path(file_path), 1)) + + +def inspect_dockerfile(file_path: Path, source_text: str, findings: list[Finding]) -> None: + """Validate Docker base image pinning and risky installer patterns.""" + for line_number, raw_line in enumerate(source_text.splitlines(), start=1): + match = DOCKER_FROM_RE.match(raw_line) + if match: + image = match.group('image') + if '@sha256:' not in image and (':' not in image.rsplit('/', 1)[-1] or image.endswith(':latest')): + add_finding( + findings, + 'Critical', + 'Blocker', + file_path, + line_number, + 'Docker base image is floating or uses latest.', + raw_line, + 'build or deploy time', + 'Pin Docker images to an immutable digest or reviewed exact version tag.', + ) + if re.search(r'curl\s+[^|]+\|\s*(?:sh|bash)|wget\s+[^|]+\|\s*(?:sh|bash)', raw_line, re.IGNORECASE): + add_finding( + findings, + 'Important', + 'Needs investigation', + file_path, + line_number, + 'Dockerfile pipes downloaded content into a shell.', + raw_line, + 'build time', + 'Download, verify, and execute installer content through a reviewed pinned checksum instead.', + ) + + +def inspect_workflow_file(file_path: Path, source_text: str, findings: list[Finding]) -> None: + """Review GitHub Actions workflow dependency and token-risk markers.""" + for line_number, raw_line in enumerate(source_text.splitlines(), start=1): + action_ref = GITHUB_ACTION_REF_RE.search(raw_line) + if action_ref: + full_ref = action_ref.group(1) + ref = action_ref.group(2) + if ref in {'main', 'master', 'latest'} or '${{' in ref: + add_finding( + findings, + 'Critical', + 'Blocker', + file_path, + line_number, + 'GitHub Action uses a floating or dynamic ref.', + raw_line, + 'CI execution path', + 'Pin actions to an immutable commit SHA or reviewed exact version.', + ) + elif not re.fullmatch(r'[0-9a-f]{40}', ref, re.IGNORECASE): + add_finding( + findings, + 'Low', + 'Acceptable with notes', + file_path, + line_number, + 'GitHub Action is version-tagged rather than pinned to an immutable commit SHA.', + full_ref, + 'CI execution path', + 'Confirm this tag is an accepted repository convention or pin to a reviewed commit SHA.', + ) + if 'pull_request_target' in raw_line: + add_finding( + findings, + 'Critical', + 'Blocker', + file_path, + line_number, + 'Workflow uses pull_request_target.', + raw_line, + 'CI execution path with elevated token risk', + 'Do not run contributor-controlled code with privileged pull_request_target tokens.', + ) + + +def inspect_dependency_manifest( + file_path: Path, + source_text: str, + findings: list[Finding], + releases: list[DependencyRelease], +) -> None: + """Apply dependency-policy checks for supported manifest types.""" + relative_path = get_relative_path(file_path) + name = file_path.name.lower() + + if is_requirement_file(relative_path): + inspect_requirements_file(file_path, source_text, findings, releases) + elif name == 'package.json': + inspect_package_json(file_path, source_text, findings, releases) + elif name == 'dockerfile': + inspect_dockerfile(file_path, source_text, findings) + elif relative_path.startswith('.github/workflows/'): + inspect_workflow_file(file_path, source_text, findings) + + +def parse_pypi_upload_time(package_name: str, version: str) -> datetime: + """Return the oldest PyPI upload time for a package version.""" + url = f'https://pypi.org/pypi/{package_name}/{version}/json' + with urlopen(url, timeout=20) as response: + payload = json.loads(response.read().decode('utf-8')) + upload_times = [item.get('upload_time_iso_8601') for item in payload.get('urls', [])] + parsed_times = [datetime.fromisoformat(value.replace('Z', '+00:00')) for value in upload_times if value] + if not parsed_times: + raise ValueError('No PyPI upload timestamps found') + return min(parsed_times) + + +def parse_npm_release_time(package_name: str, version: str) -> datetime: + """Return the npm release time for a package version.""" + if not shutil.which('npm'): + raise RuntimeError('npm is not available') + result = subprocess.run( + ['npm', 'view', f'{package_name}@{version}', 'time', '--json'], + cwd=REPO_ROOT, + check=False, + capture_output=True, + text=True, + encoding='utf-8', + errors='replace', + timeout=30, + ) + if result.returncode != 0: + raise RuntimeError(result.stderr.strip() or 'npm view failed') + payload = json.loads(result.stdout) + value = payload.get(version) if isinstance(payload, dict) else None + if not value: + raise ValueError('No npm release timestamp found') + return datetime.fromisoformat(value.replace('Z', '+00:00')) + + +def verify_release_age( + release: DependencyRelease, + min_days: int, + fail_on_unverified: bool, + findings: list[Finding], +) -> None: + """Verify that a dependency version is at least min_days old.""" + try: + if release.ecosystem == 'pypi': + release_time = parse_pypi_upload_time(release.package_name, release.version) + elif release.ecosystem == 'npm': + release_time = parse_npm_release_time(release.package_name, release.version) + else: + return + except (HTTPError, URLError, OSError, RuntimeError, TimeoutError, ValueError, json.JSONDecodeError) as exc: + severity = 'Critical' if fail_on_unverified else 'Important' + verdict = 'Blocker' if fail_on_unverified else 'Needs investigation' + add_finding( + findings, + severity, + verdict, + REPO_ROOT / release.file_path, + release.line, + f'Could not verify {release.ecosystem} release age for {release.package_name}=={release.version}.', + str(exc), + 'dependency resolution or install time', + 'Verify release metadata from the trusted registry before approving this dependency change.', + ) + return + + age_days = (datetime.now(timezone.utc) - release_time).total_seconds() / 86400 + if age_days < min_days: + add_finding( + findings, + 'Critical', + 'Blocker', + REPO_ROOT / release.file_path, + release.line, + f'{release.ecosystem} package {release.package_name}=={release.version} is newer than {min_days} full days.', + f'released {release_time.isoformat()}, age {age_days:.2f} days', + 'dependency resolution or install time', + 'Wait until the package version has aged at least seven full days or approve a documented exception.', + ) + + +def classify_path(relative_path: str) -> list[str]: + """Return risk-area labels for a changed path.""" + labels: list[str] = [] + lowered_path = relative_path.lower() + for label, markers in RISK_AREAS.items(): + for marker in markers: + marker_lower = marker.lower() + if marker_lower.startswith('.') and lowered_path.endswith(marker_lower): + labels.append(label) + break + if marker_lower in lowered_path: + labels.append(label) + break + return labels or ['other'] + + +def summarize_changed_files(paths: list[Path]) -> dict[str, list[str]]: + """Group changed files by risk area.""" + grouped: dict[str, list[str]] = {} + for path in paths: + relative_path = get_relative_path(path) + for label in classify_path(relative_path): + grouped.setdefault(label, []).append(relative_path) + return grouped + + +def format_findings(findings: list[Finding]) -> str: + """Return markdown for all findings.""" + if not findings: + return 'No malicious indicators found in the reviewed scope. This is not a safety guarantee.\n' + + severity_order = {'Critical': 0, 'Important': 1, 'Moderate': 2, 'Low': 3} + sorted_findings = sorted(findings, key=lambda item: (severity_order.get(item.severity, 99), item.file_path, item.line)) + lines: list[str] = [] + for index, finding in enumerate(sorted_findings, start=1): + location = f'{finding.file_path}:{finding.line}' if finding.line else finding.file_path + lines.extend( + [ + f'{index}. **{finding.severity} - {finding.verdict}**', + f' - Location: `{location}`', + f' - Evidence: `{finding.evidence}`', + f' - Issue: {finding.message}', + f' - Reachability: {finding.reachability}', + f' - Recommended action: {finding.recommendation}', + '', + ] + ) + return '\n'.join(lines) + + +def write_report( + report_file: Path, + paths: list[Path], + grouped_paths: dict[str, list[str]], + urls: set[str], + releases: list[DependencyRelease], + findings: list[Finding], + base_sha: str | None, + head_sha: str | None, + verify_release_age_enabled: bool, +) -> None: + """Write a markdown review report.""" + dependency_findings = [finding for finding in findings if 'dependency' in finding.reachability or 'CI execution' in finding.reachability] + lines = [ + '# Malicious PR And File Security Review', + '', + f'- Review target: `{base_sha or "working tree"}` to `{head_sha or "working tree"}`', + f'- Changed files reviewed: {len(paths)}', + f'- Dependency release-age verification: {"enabled" if verify_release_age_enabled else "skipped"}', + '', + '## Changed-File Summary', + '', + ] + for label, label_paths in sorted(grouped_paths.items()): + lines.append(f'- **{label}**: {len(label_paths)}') + for path in sorted(label_paths)[:20]: + lines.append(f' - `{path}`') + if len(label_paths) > 20: + lines.append(f' - ... {len(label_paths) - 20} more') + if not grouped_paths: + lines.append('- No changed files found.') + + lines.extend(['', '## Dependency Policy Result', '']) + if releases: + lines.append(f'- Exact dependency pins collected for release-age review: {len(releases)}') + else: + lines.append('- No direct dependency pins were collected from changed manifests.') + if dependency_findings: + lines.append(f'- Dependency or CI policy findings: {len(dependency_findings)}') + else: + lines.append('- No dependency or CI policy findings were detected in reviewed manifests.') + + lines.extend(['', '## External Hosts And URLs', '']) + if urls: + for url in sorted(urls): + lines.append(f'- `{url}`') + else: + lines.append('- No external URLs were found in reviewed changed lines.') + + lines.extend(['', '## Findings', '', format_findings(findings)]) + blockers = [finding for finding in findings if finding.verdict == 'Blocker'] + final_verdict = 'Block' if blockers else ('Needs investigation' if findings else 'No malicious indicators found in reviewed scope') + lines.extend(['', '## Final Verdict', '', final_verdict, '']) + report_file.parent.mkdir(parents=True, exist_ok=True) + report_file.write_text('\n'.join(lines), encoding='utf-8') + + +def escape_annotation_value(value: str) -> str: + """Escape GitHub Actions annotation text.""" + return value.replace('%', '%25').replace('\r', '%0D').replace('\n', '%0A').replace(':', '%3A') + + +def emit_annotations(findings: list[Finding]) -> None: + """Emit GitHub Actions annotations for findings.""" + for finding in findings: + level = 'error' if finding.verdict == 'Blocker' else 'warning' + message = f'{finding.severity} - {finding.message} Recommendation: {finding.recommendation}' + print( + f'::{level} file={finding.file_path},line={finding.line}::{escape_annotation_value(message)}' + ) + + +def run_review(args: argparse.Namespace) -> tuple[list[Finding], Path]: + """Run the static review and return findings plus report path.""" + paths = normalize_paths(args.paths) if args.paths else get_changed_files(args.base_sha, args.head_sha) + existing_paths = [path for path in paths if path.exists() and path.is_file()] + findings: list[Finding] = [] + releases: list[DependencyRelease] = [] + urls: set[str] = set() + + for file_path in existing_paths: + source_text = read_text(file_path) + if source_text is None: + inspect_binary_file(file_path, findings) + continue + + changed_lines = get_changed_lines(file_path, args.base_sha, args.head_sha) + urls.update(scan_text_patterns(file_path, source_text, changed_lines, args.full_file, findings)) + relative_path = get_relative_path(file_path) + if is_dependency_manifest(relative_path): + inspect_dependency_manifest(file_path, source_text, findings, releases) + + if args.verify_release_age: + for release in releases: + verify_release_age(release, args.min_release_age_days, args.fail_on_unverified_release_age, findings) + + report_file = Path(args.report_file) + if not report_file.is_absolute(): + report_file = REPO_ROOT / report_file + write_report( + report_file, + existing_paths, + summarize_changed_files(existing_paths), + urls, + releases, + findings, + args.base_sha, + args.head_sha, + args.verify_release_age, + ) + return findings, report_file + + +def build_parser() -> argparse.ArgumentParser: + """Build the CLI parser.""" + parser = argparse.ArgumentParser(description='Static malicious PR and file security review checker.') + parser.add_argument('paths', nargs='*', help='Explicit files to review. Defaults to git diff paths.') + parser.add_argument('--base-sha', help='Base commit for changed-line review.') + parser.add_argument('--head-sha', help='Head commit for changed-line review.') + parser.add_argument('--full-file', action='store_true', help='Scan full files instead of only changed lines.') + parser.add_argument('--verify-release-age', action='store_true', help='Query trusted registries for dependency age checks.') + parser.add_argument('--fail-on-unverified-release-age', action='store_true', help='Treat registry lookup failures as blockers.') + parser.add_argument('--min-release-age-days', type=int, default=7, help='Minimum accepted dependency version age.') + parser.add_argument('--fail-on-findings', action='store_true', help='Fail on any finding, not only blockers.') + parser.add_argument( + '--report-file', + default='artifacts/malicious-pr-security-review.md', + help='Markdown report destination.', + ) + return parser + + +def main(argv: list[str] | None = None) -> int: + """Run the CLI.""" + parser = build_parser() + args = parser.parse_args(argv) + findings, report_file = run_review(args) + emit_annotations(findings) + print(f'Malicious PR security review report written to {get_relative_path(report_file)}') + + blockers = [finding for finding in findings if finding.verdict == 'Blocker'] + if blockers: + print(f'Found {len(blockers)} blocker finding(s).') + return 1 + if args.fail_on_findings and findings: + print(f'Found {len(findings)} finding(s) and --fail-on-findings was set.') + return 1 + print('No blocker findings detected in reviewed scope.') + return 0 + + +if __name__ == '__main__': + sys.exit(main()) \ No newline at end of file