diff --git a/CHANGELOG.md b/CHANGELOG.md index f7fe19f..18c3920 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,8 +8,21 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ## [Unreleased] -### Fixed +### Added +- `--scan-all` / `--no-scan-all` CLI flags, the command-line equivalent of the + `scan_all` action input, `INPUT_SCAN_ALL` and the `scan_all` JSON/dashboard + key. `scan_all` was the one scope setting the 3.2.0 parity pass missed, so + the remediation the unresolvable-scope error recommends was not reachable + from the CLI at all. `--no-scan-all` forces the fail-closed behavior back on + for a single run when `scan_all` is already set elsewhere; passing neither + flag leaves the configured value untouched. +### Changed +- The unresolvable `changed_files` error now names the remediation for each + interface (`--scan-all`, the `scan_all` action input, `INPUT_SCAN_ALL`, a + `--config` JSON or dashboard key) instead of saying only "set scan_all". + +### Fixed - **GitHub PR notifier now reads the `github_token` notifier parameter.** The parameter is declared as `github_token` in `notifications.yaml`, which is the key the notification manager resolves dashboard configuration and the diff --git a/docs/github-action.md b/docs/github-action.md index 389a02d..5583bac 100644 --- a/docs/github-action.md +++ b/docs/github-action.md @@ -397,7 +397,7 @@ If the scope could not be applied, the run fails and says why: | `no pull request base was found` | The trigger is not `pull_request`, so there is no base. Use `changed_files: 'current-commit'` or an explicit file list | | `is not a git repository` | Run `actions/checkout` before the scan step | | `git refused to read ... even with ... safe.directory` | The checkout is damaged or incomplete. Re-run `actions/checkout`, or pass an explicit file list | -| `the scope could not be resolved` | Fix the preceding Git error, or set `scan_all: true` to opt into a full-repository fallback | +| `the scope could not be resolved` | Fix the preceding Git error, or set `scan_all: true` (CLI: `--scan-all`) to opt into a full-repository fallback | A successful empty diff is logged separately as genuinely empty and skips the scoped scanners; it is never conflated with a resolution failure. @@ -416,7 +416,10 @@ writes the runner's git config rather than the container's. `INPUT_CHANGED_FILES` environment variable, the `--changed-files` CLI flag, a `--config` JSON file, and a Socket dashboard config. `scan_all` is only the fail-open fallback when one of those requests cannot be resolved; it does not -override a successfully resolved scope. +override a successfully resolved scope. It reaches the scan the same five ways +— the `scan_all` input, `INPUT_SCAN_ALL=true`, the `--scan-all` CLI flag, a +`--config` JSON key, or dashboard config — and `--no-scan-all` turns it back +off for a single CLI run. ## PR Comment Customization @@ -852,7 +855,7 @@ Every input has a CLI flag and environment-variable equivalent; the **Scan Scope:** - `changed_files` — Diff-only mode (`auto`, `pr`, `current-commit`, a commit hash, or a file list) - `scan_files` — Explicit comma-separated file list -- `scan_all` — Fail-open fallback when a `changed_files` scope cannot be resolved +- `scan_all` — Fail-open fallback when a `changed_files` scope cannot be resolved (CLI: `--scan-all` / `--no-scan-all`) **SAST Languages:** - `all_languages_enabled` — Enable all languages diff --git a/docs/parameters.md b/docs/parameters.md index 1f1bc24..1d858c8 100644 --- a/docs/parameters.md +++ b/docs/parameters.md @@ -32,7 +32,7 @@ GitHub Action input is delivered to the container as the environment variable | `--config` | — | — | — | Path to a JSON configuration file. | | `--changed-files` | `changed_files` | `INPUT_CHANGED_FILES` | `changed_files` | Diff-only scope: `auto`, `pr`, `current-commit`, a commit hash or a file list. | | `--scan-files` | `scan_files` | `INPUT_SCAN_FILES` | `scan_files` | Explicit comma-separated file list. | -| — | `scan_all` | `INPUT_SCAN_ALL` | `scan_all` | Fail-open fallback when a `changed_files` scope cannot be resolved. | +| `--scan-all` / `--no-scan-all` | `scan_all` | `INPUT_SCAN_ALL` | `scan_all` | Fail-open fallback when a `changed_files` scope cannot be resolved. | | `--verbose`, `-v` | `verbose` | `INPUT_VERBOSE` | `verbose` | DEBUG logging. | | `--console-tabular-enabled` | `console_tabular_enabled` | `INPUT_CONSOLE_TABULAR_ENABLED` | `console_tabular_enabled` | Print consolidated findings as tables. | | `--console-json-enabled` | `console_json_enabled` | `INPUT_CONSOLE_JSON_ENABLED` | `console_json_enabled` | Print consolidated findings as JSON. | @@ -289,7 +289,9 @@ repository, git refused to read the repository, no PR base at all) and fails with a configuration error. It never reports a green scan of nothing and never silently widens to the whole repository. -Set **`scan_all`** to opt into widening on that failure path. An unresolvable +Pass **`--scan-all`** (or set `scan_all: true` as an action input, in a +`--config` JSON file or in your dashboard config, or `INPUT_SCAN_ALL=true` in +the environment) to opt into widening on that failure path. An unresolvable scope then falls back to a full-workspace scan with a warning, consistently across every enabled scanner. A scope that resolves successfully remains authoritative even when `scan_all` is set; a genuinely empty diff still skips @@ -328,6 +330,30 @@ the scanners this setting does scope are SAST/OpenGrep, secrets and containers. socket-basics --changed-files auto ``` +### `--scan-all`, `--no-scan-all` +Fail-open fallback for `--changed-files`. When the requested scope cannot be +resolved — a shallow checkout, a workspace that is not a git repository, no +pull-request base — the run normally fails with a configuration error. +`--scan-all` widens to a full-workspace scan with a warning instead, across +every enabled scanner. + +It is only a fallback: a scope that resolves successfully stays authoritative +even with `--scan-all` set, and a genuinely empty diff still skips the scoped +scanners. On its own, with no `--changed-files`, it is the default behavior — +scan the whole workspace. + +`--no-scan-all` restores the fail-closed behavior. Use it when `scan_all` is +already on from `INPUT_SCAN_ALL`, a `--config` JSON file or dashboard config +and you want this run to fail rather than silently widen. Omitting both flags +leaves whatever those sources configured untouched. + +**Example:** +```bash +# Scan the changed files; fall back to the whole workspace if git can't say +# what changed +socket-basics --changed-files auto --scan-all +``` + ### `--console-tabular-enabled` Enable consolidated console tabular output (displays results in formatted tables). diff --git a/socket_basics/core/config.py b/socket_basics/core/config.py index 2128956..84a877c 100644 --- a/socket_basics/core/config.py +++ b/socket_basics/core/config.py @@ -316,8 +316,10 @@ def _resolve_changed_files_scope(self) -> None: "changed_files: the requested scope could not be resolved, so the scan " "would either report a green run having scanned nothing or silently widen " "to the whole repository. See the warnings above for the underlying git " - "error. Fix the git problem, or set scan_all to widen to a full-repo scan " - "when the scope cannot be resolved." + "error. Fix the git problem, or opt into a full-repo scan on this failure " + "path: pass --scan-all on the CLI, set scan_all: true as a GitHub Action " + "input, set INPUT_SCAN_ALL=true in the environment, or set " + '"scan_all": true in a --config JSON file or your dashboard config.' ) self._config['changed_files'] = resolved @@ -1611,6 +1613,14 @@ def parse_cli_args(): parser.add_argument('--pull-request', type=int, help='Pull request number for full scan submission') parser.add_argument('--committers', type=str, help='Comma-separated list of committers for full scan submission') parser.add_argument('--scan-files', type=str, help='Comma-separated list of files to scan') + # Paired form rather than store_true: the default is false, but scan_all can + # already be on from INPUT_SCAN_ALL, a --config file or dashboard config, and + # store_true offers no way to turn that back off. + parser.add_argument('--scan-all', action=argparse.BooleanOptionalAction, default=None, + help="Widen to a full-workspace scan when a --changed-files scope cannot " + "be resolved, instead of failing with a configuration error. Does " + "not override a scope that resolved successfully. --no-scan-all " + "forces the fail-closed behavior back on.") parser.add_argument('--console-tabular-enabled', action='store_true', help='Enable consolidated console tabular output') parser.add_argument('--console-json-enabled', action='store_true', help='Enable consolidated console JSON output') parser.add_argument('--verbose', '-v', action='store_true', help='Verbose logging') @@ -1648,6 +1658,11 @@ def create_config_from_args(args) -> Config: config_dict['output_dir'] = args.workspace if args.scan_files: config_dict['scan_files'] = args.scan_files + # None means the flag was absent, which must not overwrite an INPUT_SCAN_ALL, + # --config JSON or dashboard value with an implicit false. + scan_all_arg = getattr(args, 'scan_all', None) + if scan_all_arg is not None: + config_dict['scan_all'] = scan_all_arg # Console tabular flag (new) with fallback to deprecated name if getattr(args, 'console_tabular_enabled', False) or getattr(args, 'output_console_enabled', False): config_dict['console_tabular_enabled'] = True diff --git a/tests/test_changed_files_scope.py b/tests/test_changed_files_scope.py index f2a5603..0f268e3 100644 --- a/tests/test_changed_files_scope.py +++ b/tests/test_changed_files_scope.py @@ -21,6 +21,7 @@ _discover_repository, _git_env, create_config_from_args, + parse_cli_args, resolve_changed_files_request, ) @@ -103,11 +104,12 @@ def _git(repo, *args): ) -def _config_args(workspace, changed_files): +def _config_args(workspace, changed_files, scan_all=None): return Namespace( config=None, workspace=str(workspace), scan_files=None, + scan_all=scan_all, console_tabular_enabled=False, output_console_enabled=False, console_json_enabled=False, @@ -376,6 +378,65 @@ def test_env_value_is_used_when_no_cli_value(self, pr_repo, monkeypatch): assert sorted(cfg.get("changed_files")) == ["base.py", "feat.py"] +class TestScanAllCliFlag: + """``--scan-all``/``--no-scan-all``: the CLI spelling of ``scan_all``. + + The error raised on an unresolvable scope recommends scan_all, so every + interface it names -- the CLI included -- has to be able to set it. + """ + + def test_flag_absent_parses_to_none(self): + assert parse_cli_args().parse_args([]).scan_all is None + + def test_scan_all_parses_to_true(self): + assert parse_cli_args().parse_args(["--scan-all"]).scan_all is True + + def test_no_scan_all_parses_to_false(self): + assert parse_cli_args().parse_args(["--no-scan-all"]).scan_all is False + + def test_flag_opts_into_the_full_scan_fallback(self, pr_repo, monkeypatch): + monkeypatch.delenv("INPUT_SCAN_ALL", raising=False) + + cfg = create_config_from_args(_config_args(pr_repo, "pr", scan_all=True)) + + assert cfg.get("scan_all") is True + assert cfg.get("changed_files") == [] + assert cfg.get("changed_files_scope_requested") is False + assert cfg.get_scan_targets() == [str(pr_repo)] + + def test_without_the_flag_an_unresolvable_scope_still_fails(self, pr_repo, monkeypatch): + monkeypatch.delenv("INPUT_SCAN_ALL", raising=False) + + with pytest.raises(SystemExit, match="could not be resolved"): + create_config_from_args(_config_args(pr_repo, "pr")) + + def test_no_scan_all_overrides_an_environment_scan_all(self, pr_repo, monkeypatch): + """The flag has to be able to turn off a value set somewhere else.""" + monkeypatch.setenv("INPUT_SCAN_ALL", "true") + + with pytest.raises(SystemExit, match="could not be resolved"): + create_config_from_args(_config_args(pr_repo, "pr", scan_all=False)) + + def test_omitting_the_flag_leaves_an_environment_scan_all_alone(self, pr_repo, monkeypatch): + monkeypatch.setenv("INPUT_SCAN_ALL", "true") + + cfg = create_config_from_args(_config_args(pr_repo, "pr")) + + assert cfg.get("scan_all") is True + assert cfg.get_scan_targets() == [str(pr_repo)] + + def test_failure_message_names_every_interface(self, pr_repo, monkeypatch): + monkeypatch.delenv("INPUT_SCAN_ALL", raising=False) + + with pytest.raises(SystemExit) as error: + create_config_from_args(_config_args(pr_repo, "pr")) + + message = str(error.value) + assert "--scan-all" in message + assert "INPUT_SCAN_ALL" in message + assert "scan_all: true" in message + + class TestPrBaseResolution: """`auto`/`pr` used to consult GITHUB_BASE_REF and nothing else."""