diff --git a/CHANGELOG.md b/CHANGELOG.md index 0fde72b..2146702 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,31 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ## [Unreleased] +### Added + +- Added `pr_comment_enabled` (default `true`). Set it to `false` to run the scan + without posting or updating the pull request comment. Scanning, the Socket + dashboard upload, `.socket.facts.json`, the high/critical job failure, and the + other notifiers are all unaffected; severity labels stay under the separate + `pr_labels_enabled` switch. +- Added `pr_comment_collapse_all` (default `false`), which collapses every + findings section including critical ones. Previously + `pr_comment_collapse_non_critical` always left critical findings expanded, so + a single critical finding forced the whole comment open. + +### Fixed + +- PR comment feature flags, including `pr_labels_enabled`, now accept string + values (`'false'`, `'0'`, `'no'`, `'off'`) as well as booleans. A Socket + dashboard config that supplied a flag as a string previously read as enabled, + because `bool("false")` is `True`. +- A blank PR comment flag now falls back to its documented default instead of + reading as off. The environment loader compared the raw value against the + single string `'true'`, so an action input forwarded from an unset workflow + variable arrived as `''` and disabled the flag, and `'1'`, `'yes'` and `'on'` + did not enable it. All the spellings the flags document now work from the + environment, a JSON config and a Socket dashboard config alike. + ## [2.2.1] - 2026-07-30 ### Fixed diff --git a/README.md b/README.md index d13d9e1..b93bae7 100644 --- a/README.md +++ b/README.md @@ -107,11 +107,12 @@ Socket Basics can also run locally or in other CI/CD environments: Socket Basics delivers **beautifully formatted, actionable PR comments** with smart defaults ā€” all enabled by default, zero configuration needed. - šŸ”— **Clickable File Links** ā€” Jump directly to the vulnerable code in GitHub -- šŸ“‹ **Collapsible Sections** ā€” Critical findings auto-expand, others collapse +- šŸ“‹ **Collapsible Sections** ā€” Critical findings auto-expand, others collapse; `pr_comment_collapse_all: 'true'` collapses critical ones too - šŸŽØ **Syntax Highlighting** ā€” Language-aware code blocks - šŸ·ļø **Auto-Labels** ā€” PRs tagged with severity-based labels (e.g., `security: critical`) - šŸ”“ **CVE Links & CVSS Scores** ā€” One-click access to NVD with risk context - šŸš€ **Full Scan Link** ā€” Report link prominently displayed at the top +- šŸ”‡ **Fully Suppressible** ā€” `pr_comment_enabled: 'false'` stops the comment entirely while the scan still runs, still uploads to the Socket dashboard, and still fails the job on high/critical findings Every feature is customizable via GitHub Actions inputs, CLI flags, or environment variables. diff --git a/action.yml b/action.yml index dcb6dc7..3a65ae4 100644 --- a/action.yml +++ b/action.yml @@ -92,9 +92,11 @@ runs: INPUT_WEBHOOK_URL: ${{ inputs.webhook_url }} SOCKET_ADDITIONAL_PARAMS: ${{ inputs.socket_additional_params }} SOCKET_TIER_1_ENABLED: ${{ inputs.socket_tier_1_enabled }} + INPUT_PR_COMMENT_ENABLED: ${{ inputs.pr_comment_enabled }} INPUT_PR_COMMENT_LINKS_ENABLED: ${{ inputs.pr_comment_links_enabled }} INPUT_PR_COMMENT_COLLAPSE_ENABLED: ${{ inputs.pr_comment_collapse_enabled }} INPUT_PR_COMMENT_COLLAPSE_NON_CRITICAL: ${{ inputs.pr_comment_collapse_non_critical }} + INPUT_PR_COMMENT_COLLAPSE_ALL: ${{ inputs.pr_comment_collapse_all }} INPUT_PR_COMMENT_CODE_FENCING_ENABLED: ${{ inputs.pr_comment_code_fencing_enabled }} INPUT_PR_COMMENT_SHOW_RULE_NAMES: ${{ inputs.pr_comment_show_rule_names }} INPUT_PR_LABELS_ENABLED: ${{ inputs.pr_labels_enabled }} @@ -443,6 +445,15 @@ inputs: description: "Generic webhook URL for WebhookNotifier" required: false default: "" + pr_comment_enabled: + description: >- + Post the findings comment on the pull request. Set to 'false' to run the + scan silently: findings are still uploaded to the Socket dashboard and the + action still fails the job on high/critical findings, but no comment is + posted or updated. Severity labels are controlled separately by + pr_labels_enabled. + required: false + default: "true" pr_comment_links_enabled: description: "Enable clickable file/line links in PR comments" required: false @@ -455,6 +466,13 @@ inputs: description: "Auto-collapse non-critical findings (critical stays expanded)" required: false default: "true" + pr_comment_collapse_all: + description: >- + Collapse every findings section, including critical ones. Use this when + you want the comment to stay small no matter what it finds. Overrides + pr_comment_collapse_non_critical. + required: false + default: "false" pr_comment_code_fencing_enabled: description: "Enable language-aware code fencing for trace output" required: false diff --git a/docs/github-action.md b/docs/github-action.md index b9ba113..b2863f8 100644 --- a/docs/github-action.md +++ b/docs/github-action.md @@ -326,6 +326,8 @@ jobs: Socket Basics automatically posts enhanced PR comments with **smart defaults that work out of the box** ā€” clickable file links, collapsible sections, syntax highlighting, CVE links, CVSS scores, and auto-labels are all enabled by default. +To run the scan without commenting on the PR at all, set `pr_comment_enabled: 'false'`. The scan still runs, findings are still uploaded to the Socket dashboard, and the job still fails on high/critical findings ā€” only the comment is suppressed. If you want a quieter comment rather than no comment, `pr_comment_collapse_all: 'true'` collapses every section including critical ones. + šŸ“– **[PR Comment Guide ā†’](github-pr-comment-guide.md)** ā€” Complete customization options, configuration examples, and reference table ## Enterprise Features diff --git a/docs/github-pr-comment-guide.md b/docs/github-pr-comment-guide.md index 640e28f..d119780 100644 --- a/docs/github-pr-comment-guide.md +++ b/docs/github-pr-comment-guide.md @@ -90,8 +90,18 @@ pr_comment_collapse_enabled: 'false' # Keep collapsible but expand everything pr_comment_collapse_enabled: 'true' pr_comment_collapse_non_critical: 'false' + +# Keep collapsible and collapse everything, critical included +pr_comment_collapse_enabled: 'true' +pr_comment_collapse_all: 'true' ``` +> [!NOTE] +> `pr_comment_collapse_non_critical` deliberately leaves critical findings +> expanded, so a single critical finding always opens the comment. Set +> `pr_comment_collapse_all: 'true'` when you want the comment to stay small no +> matter what it finds. It overrides `pr_comment_collapse_non_critical`. + --- ### 3. Syntax Highlighting (`pr_comment_code_fencing_enabled`) @@ -291,7 +301,47 @@ The logo is a 32px PNG rendered at 24x24 for retina-crisp display, with a transp --- -### 9. All-Clear Comment Updates +### 9. Turning the Comment Off (`pr_comment_enabled`) + +**Default:** `true` + +Set `pr_comment_enabled: 'false'` to run the scan without saying anything on the +PR. This is for teams who want to review finding quality in the Socket dashboard +first, without every PR growing a comment that developers have to scroll past. + +```yaml +- uses: SocketDev/socket-basics@v2 + with: + socket_security_api_key: ${{ secrets.SOCKET_SECURITY_API_KEY }} + github_token: ${{ secrets.GITHUB_TOKEN }} + pr_comment_enabled: 'false' +``` + +**What still happens when the comment is off:** + +| Behavior | Still happens? | +|----------|----------------| +| Scanners run (SAST, secrets, containers) | āœ… Yes | +| Findings uploaded to the Socket dashboard | āœ… Yes | +| `.socket.facts.json` written | āœ… Yes | +| Job fails on high/critical findings | āœ… Yes | +| Other notifiers (Slack, Jira, webhook, ...) | āœ… Yes | +| Severity labels added to the PR | āœ… Yes, unless `pr_labels_enabled: 'false'` | +| Comment posted or updated | āŒ No | + +Notifiers are the very last thing the run does ā€” the scan finishes and the +findings are uploaded to Socket before any comment would be posted ā€” so turning +the comment off cannot turn the dashboard off. Labels are a separate switch +(`pr_labels_enabled`) so you can keep or drop them independently. + +> [!TIP] +> If you want to keep the comment but make it quieter, use +> `pr_comment_collapse_all: 'true'` instead. That collapses every section, +> including critical ones, so the comment is one line until someone opens it. + +--- + +### 10. All-Clear Comment Updates When a later Socket Basics run no longer has active findings for a previously-reported scanner section, the existing PR comment section is updated in place instead of being left stale or deleted. @@ -313,9 +363,11 @@ When a later Socket Basics run no longer has active findings for a previously-re | Option | Default | Type | Description | |--------|---------|------|-------------| +| `pr_comment_enabled` | `true` | boolean | Post/update the findings comment on the PR | | `pr_comment_links_enabled` | `true` | boolean | Enable clickable file/line links | | `pr_comment_collapse_enabled` | `true` | boolean | Enable collapsible sections | -| `pr_comment_collapse_non_critical` | `true` | boolean | Auto-collapse non-critical findings | +| `pr_comment_collapse_non_critical` | `true` | boolean | Auto-collapse non-critical findings (critical stays expanded) | +| `pr_comment_collapse_all` | `false` | boolean | Collapse every section, critical included | | `pr_comment_code_fencing_enabled` | `true` | boolean | Enable syntax highlighting | | `pr_comment_show_rule_names` | `true` | boolean | Show explicit rule names | | `pr_labels_enabled` | `true` | boolean | Add severity-based labels to PRs | @@ -324,6 +376,18 @@ When a later Socket Basics run no longer has active findings for a previously-re | `pr_label_medium` | `"security: medium"` | string | Label name for medium findings | | `pr_label_low` | `"security: low"` | string | Label name for low findings | +### How boolean options are read + +Every boolean above accepts `true`, `1`, `yes` and `on` for on, and `false`, +`0`, `no` and `off` for off, in any capitalization, whether it arrives as a +GitHub Action input, an environment variable, a `--config` JSON file, or a +Socket dashboard config. + +A value that says nothing ā€” blank, whitespace, or a word that is neither ā€” falls +back to the default in the table. This matters for `pr_comment_enabled`: passing +it a workflow variable that turns out to be unset gives the action an empty +string, and that leaves the comment on rather than silently switching it off. + ### Configuration Methods **1. GitHub Actions (Recommended)** @@ -395,6 +459,21 @@ pr_label_high: 'security' pr_label_medium: 'security' ``` +### Evaluation / Trial (Dashboard Only) + +Review findings in the Socket dashboard without putting anything on the PR: +```yaml +pr_comment_enabled: 'false' +pr_labels_enabled: 'false' +``` + +### Quiet Comment (Everything Collapsed) + +Keep a single collapsed comment even when there are critical findings: +```yaml +pr_comment_collapse_all: 'true' +``` + --- ## šŸš€ Migration Guide diff --git a/scripts/preview_pr_comments.py b/scripts/preview_pr_comments.py index a7e6d55..edb761a 100644 --- a/scripts/preview_pr_comments.py +++ b/scripts/preview_pr_comments.py @@ -36,6 +36,7 @@ def make_mock_config( repo="SocketDev/example-app", commit="a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2", full_scan_url="https://socket.dev/dashboard/scan/12345", + collapse_all=False, ): """Build a mock config object matching the real pipeline shape.""" return MockConfig( @@ -44,6 +45,7 @@ def make_mock_config( pr_comment_links_enabled=True, pr_comment_collapse_enabled=True, pr_comment_collapse_non_critical=True, + pr_comment_collapse_all=collapse_all, pr_comment_code_fencing_enabled=True, pr_comment_show_rule_names=True, full_scan_html_url=full_scan_url, diff --git a/socket_basics/core/config.py b/socket_basics/core/config.py index ac623b4..b3ec0df 100644 --- a/socket_basics/core/config.py +++ b/socket_basics/core/config.py @@ -14,6 +14,38 @@ logger = logging.getLogger(__name__) +TRUE_STRINGS = ('true', '1', 'yes', 'on') +FALSE_STRINGS = ('false', '0', 'no', 'off') + + +def coerce_bool(value: Any, default: bool) -> bool: + """Coerce a config value to a bool, tolerating the string forms. + + Every layer that carries a flag delivers it differently: the environment + loader sees strings because GitHub Action inputs are always strings, a + Socket dashboard config can send either, and a JSON config sends real + booleans. ``bool("false")`` is ``True``, so a plain cast turns a disabled + flag back on. + + A value that says nothing -- ``None``, an empty string, or a word that is + neither true nor false -- resolves to ``default`` rather than to ``False``. + An action input forwarded from an unset workflow variable arrives as an + empty string, and reading that as "off" silently disables a flag that + nobody asked to disable. + """ + if value is None: + return default + if isinstance(value, bool): + return value + if isinstance(value, str): + normalized = value.strip().lower() + if normalized in TRUE_STRINGS: + return True + if normalized in FALSE_STRINGS: + return False + return default + return bool(value) + def _normalize_path_parts(path_value: str | None) -> List[str] | None: """Normalize a path-like string into comparable POSIX-style path segments.""" @@ -812,7 +844,7 @@ def load_config_from_env() -> Dict[str, Any]: env_value = os.getenv(env_var) if env_value is not None: if p_type == 'bool': - config[p_name] = env_value.lower() == 'true' + config[p_name] = coerce_bool(env_value, bool(default_value)) elif p_type == 'int': try: config[p_name] = int(env_value) diff --git a/socket_basics/core/connector/opengrep/github_pr.py b/socket_basics/core/connector/opengrep/github_pr.py index 36171ad..9d869ae 100644 --- a/socket_basics/core/connector/opengrep/github_pr.py +++ b/socket_basics/core/connector/opengrep/github_pr.py @@ -34,6 +34,7 @@ def format_notifications(groups: Dict[str, List[Dict[str, Any]]], config=None) - enable_links = flags['enable_links'] enable_collapse = flags['enable_collapse'] collapse_non_critical = flags['collapse_non_critical'] + collapse_all = flags['collapse_all'] enable_code_fencing = flags['enable_code_fencing'] show_rule_names = flags['show_rule_names'] repository = flags['repository'] @@ -202,8 +203,10 @@ def format_notifications(groups: Dict[str, List[Dict[str, Any]]], config=None) - if enable_collapse: # Determine if this should be auto-expanded has_critical = file_severities['critical'] > 0 - # Auto-expand if: no collapse requested OR has critical findings - auto_expand = (not collapse_non_critical) or has_critical + # Auto-expand if: no collapse requested OR has critical findings. + # collapse_all wins over both so the comment can stay small + # even when a critical finding is present. + auto_expand = ((not collapse_non_critical) or has_critical) and not collapse_all collapsible = helpers.create_collapsible_section( display_path, # Don't use backticks in summary - they don't render in GitHub diff --git a/socket_basics/core/connector/socket_tier1/github_pr.py b/socket_basics/core/connector/socket_tier1/github_pr.py index 0e33741..9fd48aa 100644 --- a/socket_basics/core/connector/socket_tier1/github_pr.py +++ b/socket_basics/core/connector/socket_tier1/github_pr.py @@ -34,6 +34,7 @@ def format_notifications(components_list: List[Dict[str, Any]], config=None) -> enable_links = flags['enable_links'] enable_collapse = flags['enable_collapse'] collapse_non_critical = flags['collapse_non_critical'] + collapse_all = flags['collapse_all'] enable_code_fencing = flags['enable_code_fencing'] show_rule_names = flags['show_rule_names'] repository = flags['repository'] @@ -154,7 +155,10 @@ def format_notifications(components_list: List[Dict[str, Any]], config=None) -> severity_summary = " | ".join(severity_parts) if severity_parts else "No issues" - open_attr = ' open' if (not collapse_non_critical or has_critical) else '' + # collapse_all wins over both so the comment can stay small even + # when a critical finding is present. + auto_expand = (not collapse_non_critical or has_critical) and not collapse_all + open_attr = ' open' if auto_expand else '' content_lines.append(f"") content_lines.append(f"{purl} ({severity_summary})") content_lines.append("") diff --git a/socket_basics/core/notification/github_pr_helpers.py b/socket_basics/core/notification/github_pr_helpers.py index d678fc6..311e70c 100644 --- a/socket_basics/core/notification/github_pr_helpers.py +++ b/socket_basics/core/notification/github_pr_helpers.py @@ -14,6 +14,10 @@ from typing import Dict, Any, Optional, List, Tuple from pathlib import Path +# coerce_bool lives in the config layer so the environment loader, a Socket +# dashboard config and these flags all read a string the same way. +from socket_basics.core.config import coerce_bool + # ============================================================================ # Severity Constants (shared across all scanners) @@ -99,6 +103,7 @@ def get_feature_flags(config) -> Dict[str, Any]: 'enable_links': True, 'enable_collapse': True, 'collapse_non_critical': True, + 'collapse_all': False, 'enable_code_fencing': True, 'show_rule_names': True, 'repository': '', @@ -107,11 +112,12 @@ def get_feature_flags(config) -> Dict[str, Any]: } return { - 'enable_links': config.get('pr_comment_links_enabled', True), - 'enable_collapse': config.get('pr_comment_collapse_enabled', True), - 'collapse_non_critical': config.get('pr_comment_collapse_non_critical', True), - 'enable_code_fencing': config.get('pr_comment_code_fencing_enabled', True), - 'show_rule_names': config.get('pr_comment_show_rule_names', True), + 'enable_links': coerce_bool(config.get('pr_comment_links_enabled'), True), + 'enable_collapse': coerce_bool(config.get('pr_comment_collapse_enabled'), True), + 'collapse_non_critical': coerce_bool(config.get('pr_comment_collapse_non_critical'), True), + 'collapse_all': coerce_bool(config.get('pr_comment_collapse_all'), False), + 'enable_code_fencing': coerce_bool(config.get('pr_comment_code_fencing_enabled'), True), + 'show_rule_names': coerce_bool(config.get('pr_comment_show_rule_names'), True), 'repository': config.repo if hasattr(config, 'repo') else '', 'commit_hash': config.commit_hash if hasattr(config, 'commit_hash') else '', 'full_scan_url': config.get('full_scan_html_url') if config else None diff --git a/socket_basics/core/notification/github_pr_notifier.py b/socket_basics/core/notification/github_pr_notifier.py index a87e1ea..a7bcca4 100644 --- a/socket_basics/core/notification/github_pr_notifier.py +++ b/socket_basics/core/notification/github_pr_notifier.py @@ -3,6 +3,7 @@ from urllib.parse import quote from socket_basics.core.notification.base import BaseNotifier +from socket_basics.core.notification.github_pr_helpers import coerce_bool from socket_basics.core.config import get_github_token, get_github_repository, get_github_pr_number logger = logging.getLogger(__name__) @@ -36,8 +37,9 @@ def __init__(self, params: Dict[str, Any] | None = None): def notify(self, facts: Dict[str, Any]) -> None: notifications = facts.get('notifications', []) or [] - labels_enabled = self.config.get('pr_labels_enabled', True) - + labels_enabled = coerce_bool(self.config.get('pr_labels_enabled'), True) + comment_enabled = coerce_bool(self.config.get('pr_comment_enabled'), True) + if not isinstance(notifications, list): logger.error('GithubPRNotifier: only supports new format - list of dicts with title/content') return @@ -57,6 +59,24 @@ def notify(self, facts: Dict[str, Any]) -> None: notification_section_types = self._extract_section_types_from_notifications(valid_notifications) facts_section_types = self._infer_section_types_from_facts(facts) + # Comment suppression: the scan has already run and the findings have + # already been uploaded to the Socket dashboard by the time notifiers + # execute, so this only silences the PR comment. Severity labels stay + # under pr_labels_enabled so the two can be turned off independently. + if not comment_enabled: + logger.info( + 'GithubPRNotifier: PR comments disabled (pr_comment_enabled=false); ' + '%d finding section(s) were scanned and uploaded to the Socket dashboard ' + 'but no comment will be posted or updated', + len(valid_notifications), + ) + if labels_enabled: + pr_number = self._get_pr_number() + if pr_number: + labels = self._determine_pr_labels(valid_notifications) + self._reconcile_pr_labels(pr_number, labels) + return + if not valid_notifications: pr_number = self._get_pr_number() if pr_number: diff --git a/socket_basics/notifications.yaml b/socket_basics/notifications.yaml index 02c2e3d..4782108 100644 --- a/socket_basics/notifications.yaml +++ b/socket_basics/notifications.yaml @@ -90,6 +90,12 @@ notifiers: option: --github-api-url env_variable: GITHUB_API_URL type: str + - name: pr_comment_enabled + option: --pr-comment + env_variable: INPUT_PR_COMMENT_ENABLED + type: bool + default: true + description: "Post/update the findings comment on the PR (scanning and dashboard upload are unaffected)" - name: pr_comment_links_enabled option: --pr-comment-links env_variable: INPUT_PR_COMMENT_LINKS_ENABLED @@ -108,6 +114,12 @@ notifiers: type: bool default: true description: "Auto-collapse non-critical findings (critical stays expanded)" + - name: pr_comment_collapse_all + option: --pr-comment-collapse-all + env_variable: INPUT_PR_COMMENT_COLLAPSE_ALL + type: bool + default: false + description: "Collapse every findings section, including critical ones" - name: pr_comment_code_fencing_enabled option: --pr-comment-code-fencing env_variable: INPUT_PR_COMMENT_CODE_FENCING_ENABLED diff --git a/tests/test_github_pr_notifier.py b/tests/test_github_pr_notifier.py index 196e150..6752716 100644 --- a/tests/test_github_pr_notifier.py +++ b/tests/test_github_pr_notifier.py @@ -347,3 +347,140 @@ def test_notify_zero_alert_enabled_sast_config_rewrites_matching_section(monkeyp assert len(updated_bodies) == 1 assert 'Socket Basics found no active findings in the latest run.' in updated_bodies[0] assert '' in updated_bodies[0] + + +# --------------------------------------------------------------------------- +# pr_comment_enabled: suppress the PR comment without suppressing the scan +# --------------------------------------------------------------------------- + +def _suppression_notifier(monkeypatch, **params): + """A notifier whose every GitHub write is recorded instead of performed.""" + notifier = GithubPRNotifier({'repository': 'SocketDev/socket-basics', **params}) + calls = {'posted': [], 'updated': [], 'labels': []} + + monkeypatch.setattr(notifier, '_get_pr_number', lambda: 123) + monkeypatch.setattr(notifier, '_get_pr_comments', lambda pr_number: []) + monkeypatch.setattr( + notifier, '_post_comment', lambda pr_number, body: calls['posted'].append(body) or True + ) + monkeypatch.setattr( + notifier, + '_update_comment', + lambda pr_number, comment_id, body: calls['updated'].append(body) or True, + ) + monkeypatch.setattr( + notifier, + '_reconcile_pr_labels', + lambda pr_number, labels: calls['labels'].append(labels) or True, + ) + return notifier, calls + + +_FINDING = { + 'title': 'Socket SAST JavaScript', + 'content': '\nšŸ”“ Critical: 1\n', +} + + +def test_notify_posts_comment_by_default(monkeypatch): + notifier, calls = _suppression_notifier(monkeypatch) + + notifier.notify({'notifications': [_FINDING], 'components': []}) + + assert len(calls['posted']) == 1 + + +def test_notify_posts_no_comment_when_pr_comment_disabled(monkeypatch): + notifier, calls = _suppression_notifier(monkeypatch, pr_comment_enabled=False) + + notifier.notify({'notifications': [_FINDING], 'components': []}) + + assert calls['posted'] == [] + assert calls['updated'] == [] + + +def test_notify_posts_no_all_clear_comment_when_pr_comment_disabled(monkeypatch): + """An empty run must not rewrite an existing comment into "no findings".""" + existing = """ +## Socket SAST JavaScript + +### Summary +šŸŸ  High: 1 +""" + + notifier, calls = _suppression_notifier(monkeypatch, pr_comment_enabled=False) + notifier.app_config = {'javascript_sast_enabled': True} + monkeypatch.setattr(notifier, '_get_pr_comments', lambda pr_number: [{'id': 99, 'body': existing}]) + + notifier.notify({'notifications': [], 'components': []}) + + assert calls['posted'] == [] + assert calls['updated'] == [] + + +def test_notify_still_applies_labels_when_pr_comment_disabled(monkeypatch): + """Comments and labels are independent switches.""" + notifier, calls = _suppression_notifier( + monkeypatch, pr_comment_enabled=False, pr_labels_enabled=True + ) + + notifier.notify({'notifications': [_FINDING], 'components': []}) + + assert calls['labels'] == [['security: critical']] + + +def test_notify_skips_labels_when_both_switches_are_off(monkeypatch): + notifier, calls = _suppression_notifier( + monkeypatch, pr_comment_enabled=False, pr_labels_enabled=False + ) + + notifier.notify({'notifications': [_FINDING], 'components': []}) + + assert calls['posted'] == [] + assert calls['labels'] == [] + + +def test_notify_honors_string_false_from_dashboard_config(monkeypatch): + """A Socket dashboard config supplies flags as strings, not booleans.""" + notifier, calls = _suppression_notifier(monkeypatch, pr_comment_enabled='false') + + notifier.notify({'notifications': [_FINDING], 'components': []}) + + assert calls['posted'] == [] + + +def test_notify_honors_string_false_for_labels(monkeypatch): + """pr_labels_enabled needs the same string handling as pr_comment_enabled.""" + notifier, calls = _suppression_notifier( + monkeypatch, pr_comment_enabled='false', pr_labels_enabled='false' + ) + + notifier.notify({'notifications': [_FINDING], 'components': []}) + + assert calls['posted'] == [] + assert calls['labels'] == [] + + +def test_notify_string_false_labels_are_skipped_on_the_normal_path(monkeypatch): + """Not just the suppression branch -- the ordinary posting path too.""" + notifier, calls = _suppression_notifier(monkeypatch, pr_labels_enabled='false') + + notifier.notify({'notifications': [_FINDING], 'components': []}) + + assert len(calls['posted']) == 1 + assert calls['labels'] == [] + + +def test_notify_leaves_facts_untouched_when_pr_comment_disabled(monkeypatch): + """Suppression is comment-only: the uploaded facts payload is not altered.""" + notifier, _calls = _suppression_notifier(monkeypatch, pr_comment_enabled=False) + facts = { + 'notifications': [_FINDING], + 'components': [{'id': 'src/index.js', 'alerts': [{'severity': 'critical'}]}], + 'full_scan_html_url': 'https://socket.dev/dashboard/scan/12345', + } + + notifier.notify(facts) + + assert facts['components'] == [{'id': 'src/index.js', 'alerts': [{'severity': 'critical'}]}] + assert facts['full_scan_html_url'] == 'https://socket.dev/dashboard/scan/12345' diff --git a/tests/test_pr_formatters.py b/tests/test_pr_formatters.py index 564ad0a..31c12ba 100644 --- a/tests/test_pr_formatters.py +++ b/tests/test_pr_formatters.py @@ -356,3 +356,132 @@ def test_tier1_empty_list(self, config): results = format_notifications([], config=config) assert len(results) == 1 assert "No reachability issues found" in results[0]["content"] + + +# --------------------------------------------------------------------------- +# pr_comment_collapse_all: keep every section collapsed, critical included +# --------------------------------------------------------------------------- + +@pytest.fixture +def config_collapse_all(): + return make_mock_config(collapse_all=True) + + +class TestCollapseAll: + """`pr_comment_collapse_all` is the only way to collapse critical findings. + + `pr_comment_collapse_non_critical` deliberately leaves critical sections + expanded, so before this flag existed a critical finding always forced the + comment open. + """ + + def test_opengrep_critical_section_is_expanded_by_default(self, config): + from socket_basics.core.connector.opengrep.github_pr import format_notifications + results = format_notifications(OPENGREP_FIXTURES, config=config) + js_content = next(r["content"] for r in results if "JavaScript" in r["title"]) + assert "
" in js_content + + def test_opengrep_collapses_critical_section_when_enabled(self, config_collapse_all): + from socket_basics.core.connector.opengrep.github_pr import format_notifications + results = format_notifications(OPENGREP_FIXTURES, config=config_collapse_all) + js_content = next(r["content"] for r in results if "JavaScript" in r["title"]) + assert "
" not in js_content + # Sections are still present, just closed. + assert "
" in js_content + + def test_tier1_critical_section_is_expanded_by_default(self, config): + from socket_basics.core.connector.socket_tier1.github_pr import format_notifications + content = format_notifications(TIER1_FIXTURES, config=config)[0]["content"] + assert "
" in content + + def test_tier1_collapses_critical_section_when_enabled(self, config_collapse_all): + from socket_basics.core.connector.socket_tier1.github_pr import format_notifications + content = format_notifications(TIER1_FIXTURES, config=config_collapse_all)[0]["content"] + assert "
" not in content + assert "
" in content + + +class TestFeatureFlagCoercion: + """Dashboard-sourced config can deliver flags as strings, not booleans.""" + + def test_string_true_enables_collapse_all(self): + from socket_basics.core.notification.github_pr_helpers import get_feature_flags + flags = get_feature_flags(make_mock_config(collapse_all="true")) + assert flags["collapse_all"] is True + + def test_string_false_does_not_enable_collapse_all(self): + from socket_basics.core.notification.github_pr_helpers import get_feature_flags + flags = get_feature_flags(make_mock_config(collapse_all="false")) + assert flags["collapse_all"] is False + + def test_string_false_disables_links(self): + from socket_basics.core.notification.github_pr_helpers import get_feature_flags + config = make_mock_config() + config["pr_comment_links_enabled"] = "false" + assert get_feature_flags(config)["enable_links"] is False + + def test_missing_config_keeps_documented_defaults(self): + from socket_basics.core.notification.github_pr_helpers import get_feature_flags + flags = get_feature_flags(None) + assert flags["collapse_all"] is False + assert flags["collapse_non_critical"] is True + + +class TestActionInputCoercion: + """GitHub Action inputs are always strings, and blank means "unset". + + An input forwarded from an unset workflow variable arrives as an empty + string. Reading that as "off" would silently suppress the PR comment for a + workflow that never asked to suppress it, so a value that says nothing has + to fall back to the documented default. + """ + + def _load(self, monkeypatch, **env): + from socket_basics.core.config import load_config_from_env + for name in ( + "INPUT_PR_COMMENT_ENABLED", + "INPUT_PR_COMMENT_COLLAPSE_ALL", + "INPUT_PR_LABELS_ENABLED", + ): + monkeypatch.delenv(name, raising=False) + for name, value in env.items(): + monkeypatch.setenv(name, value) + return load_config_from_env() + + def test_comment_stays_enabled_when_the_input_is_unset(self, monkeypatch): + assert self._load(monkeypatch).get("pr_comment_enabled") is True + + def test_comment_stays_enabled_for_a_blank_input(self, monkeypatch): + config = self._load(monkeypatch, INPUT_PR_COMMENT_ENABLED="") + assert config.get("pr_comment_enabled") is True + + def test_comment_stays_enabled_for_a_whitespace_input(self, monkeypatch): + config = self._load(monkeypatch, INPUT_PR_COMMENT_ENABLED=" ") + assert config.get("pr_comment_enabled") is True + + def test_comment_stays_enabled_for_an_unrecognized_input(self, monkeypatch): + config = self._load(monkeypatch, INPUT_PR_COMMENT_ENABLED="maybe") + assert config.get("pr_comment_enabled") is True + + @pytest.mark.parametrize("value", ["false", "False", "FALSE", "0", "no", "off"]) + def test_comment_is_disabled_by_every_false_spelling(self, monkeypatch, value): + config = self._load(monkeypatch, INPUT_PR_COMMENT_ENABLED=value) + assert config.get("pr_comment_enabled") is False + + @pytest.mark.parametrize("value", ["true", "True", "TRUE", "1", "yes", "on"]) + def test_comment_is_enabled_by_every_true_spelling(self, monkeypatch, value): + config = self._load(monkeypatch, INPUT_PR_COMMENT_ENABLED=value) + assert config.get("pr_comment_enabled") is True + + def test_collapse_all_stays_off_for_a_blank_input(self, monkeypatch): + """A blank input falls back to the default, which here is off.""" + config = self._load(monkeypatch, INPUT_PR_COMMENT_COLLAPSE_ALL="") + assert config.get("pr_comment_collapse_all") is False + + def test_collapse_all_is_enabled_by_a_string_true(self, monkeypatch): + config = self._load(monkeypatch, INPUT_PR_COMMENT_COLLAPSE_ALL="true") + assert config.get("pr_comment_collapse_all") is True + + def test_labels_stay_enabled_for_a_blank_input(self, monkeypatch): + config = self._load(monkeypatch, INPUT_PR_LABELS_ENABLED="") + assert config.get("pr_labels_enabled") is True