diff --git a/.github/actions/keywatch-scan/action.yml b/.github/actions/keywatch-scan/action.yml index 6de3401..3860188 100644 --- a/.github/actions/keywatch-scan/action.yml +++ b/.github/actions/keywatch-scan/action.yml @@ -23,6 +23,10 @@ inputs: description: 'Deprecated: verbose scanner output is disabled to avoid logging matched secrets' required: false default: 'false' + config: + description: 'Path to an explicit trusted .keywatch.toml config file' + required: false + default: '' outputs: findings-count: @@ -133,6 +137,7 @@ runs: INPUT_EXIT_MODE: ${{ inputs.exit-mode }} INPUT_OUTPUT: ${{ inputs.output }} INPUT_VERBOSE: ${{ inputs.verbose }} + INPUT_CONFIG: ${{ inputs.config }} run: | set -euo pipefail @@ -205,7 +210,7 @@ runs: if ((${#extra_args[@]})); then for arg in "${extra_args[@]}"; do case "$arg" in - --verbose|--verbose=*|-v*|--format|--format=*|-f|-f*|--output|--output=*|-o|-o*|--exit-mode|--exit-mode=*) + --verbose|--verbose=*|-v*|--format|--format=*|-f|-f*|--output|--output=*|-o|-o*|--exit-mode|--exit-mode=*|--config|--config=*|--no-config-discovery|--no-config-discovery=*) echo "ERROR: '$arg' is managed by action inputs and cannot be passed through args" >&2 exit 1 ;; @@ -224,8 +229,11 @@ runs: mkdir -p "$(dirname -- "$report_path")" fi - keywatch_args=(scan) + keywatch_args=(scan --no-config-discovery) keywatch_args+=(--exit-mode "$INPUT_EXIT_MODE") + if [ -n "$INPUT_CONFIG" ]; then + keywatch_args+=(--config "$INPUT_CONFIG") + fi if ((${#extra_args[@]})); then keywatch_args+=("${extra_args[@]}") fi diff --git a/.pre-commit-hooks.yaml b/.pre-commit-hooks.yaml new file mode 100644 index 0000000..63157c1 --- /dev/null +++ b/.pre-commit-hooks.yaml @@ -0,0 +1,19 @@ +- id: keywatch-scan + name: KeyWatch scan + description: Scan staged files for secrets, API keys, tokens, and credentials + entry: key-watch scan + language: rust + files: '' + pass_filenames: true + verbose: false + args: ['--exit-mode=strict'] + +- id: keywatch-scan-system + name: KeyWatch scan (system) + description: Scan staged files for secrets, API keys, tokens, and credentials + entry: key-watch scan + language: system + files: '' + pass_filenames: true + verbose: false + args: ['--exit-mode=strict'] diff --git a/CHANGELOG.md b/CHANGELOG.md index 60c31ab..24179cc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,10 @@ All notable changes to this project will be documented in this file. ## [Unreleased] +### Breaking Changes + +- Public Rust APIs now return module-local typed errors instead of `String` or boxed errors. This affects CLI validation, baseline, configuration, detector initialization, scanner, hook, and `run_cli()` return types and requires a major-version release. + ### Added - **CRITICAL severity support** — findings can now be scored as Critical, High, Medium, or Low @@ -19,9 +23,15 @@ All notable changes to this project will be documented in this file. - **GitHub Action** — composite action (`action.yml`) for CI/CD integration - **Docker support** — multi-stage Dockerfile with `--locked` flag, stripped binary, non-root user, and git installed for `--git-history` scanning and hook installation - `.dockerignore` for optimized Docker builds +- **Config file support** — `.keywatch.toml` with custom rules, detector overrides, and exclude patterns +- **SARIF 2.1.0 output** — `--format sarif` enables GitHub Code Scanning and SARIF viewer integration +- **`--config` CLI flag** — specify a custom path to `.keywatch.toml` +- **Pre-commit `language: system`** — generated hooks use `language: system` for faster execution ### Changed +- `get_severity_counts()` now returns 4-tuple (Critical, High, Medium, Low) instead of 3-tuple +- `run_scan()` accepts optional `config` parameter for merging user configuration - Simplified distribution to a single shipped binary: `key-watch` - Git hook installation now supports first-class global hooks via `core.hooksPath` - Installation guidance is now cargo-first, with manual GitHub Releases setup documented step by step @@ -49,6 +59,10 @@ All notable changes to this project will be documented in this file. - `scripts/install.sh` in favor of documented `cargo install` and manual release-binary setup - ~1650 lines of redundant context-based detectors; kept only prefix-based detectors plus GenericKeyValueDetector +### Documentation + +- README architecture documentation now uses two inline GitHub-compatible Mermaid diagrams for the system overview and detection pipeline, alongside the layer-by-layer overview and core data type reference + ## [1.1.0] - 2026-05-05 ### Added diff --git a/README.md b/README.md index 2a6e1f4..521d77e 100644 --- a/README.md +++ b/README.md @@ -123,6 +123,9 @@ key-watch verify-integrity ## Options - `scan ...` - Scan one or more files or directories +- `scan --config ` - Load configuration from an explicit `.keywatch.toml` path +- `scan --no-config-discovery` - Ignore discovered repository config unless `--config` is explicit +- `scan --format ` - Choose the report format written to stdout or the output file - `scan --stdin` - Read content from stdin instead of files - `scan --git-history` - Scan git history (`git log -p`) for committed secrets - `scan --output ` - Save report to file @@ -191,6 +194,121 @@ password = 'known-test-password' # keywatch:ignore - KeyWatch refuses to overwrite a non-KeyWatch global hook file - KeyWatch also refuses to remove a non-KeyWatch global hook file +## Architecture + +### System Overview + +```mermaid +flowchart TD + CLI["key-watch CLI"] + Scan["scan command"] + Hooks["hook install / uninstall"] + Setup["init / verify-integrity"] + + Sources["Scan sources
files, directories, stdin, or git history"] + BuiltIns["detectors.toml
built-in rules"] + UserConfig[".keywatch.toml or --config
custom rules, overrides, excludes"] + Detectors["Merged detector set"] + Pipeline["Detection pipeline"] + Findings["Findings + ScanMetadata"] + BaselineAction{"Baseline action"} + BaselineFilter["Filter known findings
--baseline"] + BaselineUpdate["Write updated baseline
--update-baseline"] + BaselineFile["Baseline JSON + exit"] + Report["JSON or SARIF 2.1.0 report"] + Destination["stdout or --output
summary + exit code"] + + HookTargets["Git hook targets
local or global"] + PreCommit["pre-commit
scan staged files"] + PrePush["pre-push
check policy, then scan repository"] + + CLI --> Scan + CLI --> Hooks + CLI --> Setup + + Hooks --> HookTargets + HookTargets --> PreCommit + HookTargets --> PrePush + PreCommit --> Scan + PrePush --> Scan + + Scan --> Sources + Scan --> BuiltIns + Scan --> UserConfig + BuiltIns --> Detectors + UserConfig --> Detectors + Sources --> Pipeline + Detectors --> Pipeline + Pipeline --> Findings + Findings --> BaselineAction + BaselineAction -->|none| Report + BaselineAction -->|filter| BaselineFilter + BaselineAction -->|update| BaselineUpdate + BaselineFilter --> Report + BaselineUpdate --> BaselineFile + Report --> Destination +``` + +### Architecture Overview + +KeyWatch is organized into five layers. Data flows top to bottom: input sources and configuration feed the detection pipeline, findings pass through post-processing, and results are serialized to stdout or a file. + +1. **Input** — the CLI accepts files, directories, stdin, or git history (`--git-history`). Flags control exclusion (`--exclude`), baselineing (`--baseline`, `--update-baseline`), output format (`--format`), config path (`--config`), and exit behavior (`--exit-mode`). +2. **Configuration** — `detectors.toml` ships with the binary and holds the built-in rules. An optional `.keywatch.toml` adds custom rules, per-detector severity/enable overrides, and exclude patterns. Configuration merges — it never replaces defaults. +3. **Detection pipeline** — six stages run per file: collect files (recursive walk, skipping `.git` and binary files), apply exclude globs, pre-filter by keyword (fast path that avoids regex on irrelevant files), match regexes (single-line and multiline `(?s)`), gate on Shannon entropy, and apply allowlists plus inline `keywatch:ignore` suppression. Files are scanned in parallel with rayon. +4. **Post-processing** — an optional baseline filter suppresses findings already recorded in the baseline file, keyed by a salted SHA-256 fingerprint of the matched content. +5. **Output** — findings serialize as JSON or SARIF 2.1.0 and are written to stdout or an output file, followed by a severity summary and an exit code derived from the exit mode. + +### Detection Pipeline + +```mermaid +flowchart TD + Start["scan command"] + Config["Load optional configuration"] + Detectors["Initialize built-in and custom detectors"] + Mode{"Input mode"} + + Paths["Files or directories"] + Stdin["stdin stream"] + History["git log patch stream"] + + Collect["Collect targets
recursive walk, skip symlinks and .git"] + Dedupe["Sort and deduplicate targets"] + Exclude["Apply CLI and config exclude globs"] + Read["Read text files
skip binary and non-UTF-8 content"] + Parallel["Scan files in parallel with rayon"] + Stream["Scan stream in overlapping chunks"] + + Keyword["Keyword pre-filter"] + Regex["Line and multiline regex matching"] + Entropy["Entropy threshold"] + Suppress["Detector allowlist + inline suppression"] + Emit["Emit Finding"] + Result["Return findings + metadata"] + + Start --> Config --> Detectors --> Mode + Mode -->|paths| Paths + Mode -->|stdin| Stdin + Mode -->|git history| History + + Paths --> Collect --> Dedupe --> Exclude --> Read --> Parallel + Stdin --> Stream + History --> Stream + + Parallel --> Keyword + Stream --> Keyword + Keyword --> Regex --> Entropy --> Suppress --> Emit --> Result +``` + +### Core Data Types + +- **Detector** — a named rule: regex, finding type, severity, optional keywords for pre-filtering, an entropy threshold, and an allowlist. +- **Finding** — one detected secret: file path, line number, finding type, severity, matched content, and the detector that produced it. +- **Severity** — `Critical`, `High`, `Medium`, `Low`. +- **KeywatchConfig** — parsed `.keywatch.toml`: custom rules, per-detector overrides, and exclude patterns. +- **Baseline** — versioned collection of fingerprint entries; filters out already-known findings. +- **ScanMetadata** — files scanned, total lines, and excluded files, reported alongside findings. + ## Development ```sh diff --git a/scripts/validate-keywatch-action.py b/scripts/validate-keywatch-action.py index f99fd36..f8946a3 100755 --- a/scripts/validate-keywatch-action.py +++ b/scripts/validate-keywatch-action.py @@ -27,6 +27,7 @@ class ScanScenario(NamedTuple): expected_stderr: tuple[str, ...] = () expected_summary: tuple[str, ...] = () preseed_report: bool = False + config: str = "" def run_blocks(text: str) -> list[str]: @@ -74,10 +75,12 @@ def write_keywatch_stub(bin_dir: Path) -> Path: def run_scan_scenarios(scan_block: str) -> None: scenarios = ( - ScanScenario("glob-expands", "scan/*.txt", "", 0, "valid", 0, ("exit_code=0", "findings_count=2"), ("scan/match.txt",), ("scan/*.txt", "--verbose")), + ScanScenario("glob-expands", "scan/*.txt", "", 0, "valid", 0, ("exit_code=0", "findings_count=2"), ("--no-config-discovery", "scan/match.txt"), ("scan/*.txt", "--verbose")), + ScanScenario("explicit-config", ".", "", 0, "valid", 0, expected_capture=("--no-config-discovery\n", "--config\ntrusted.toml\n"), config="trusted.toml"), ScanScenario("semicolon-literal", "literal;touch_pwned", "", 0, "valid", 0, expected_capture=("literal;touch_pwned",)), ScanScenario("path-option-is-literal", "--verbose", "", 0, "valid", 0, expected_capture=("--\n--verbose\n",)), ScanScenario("format-long-value-rejected", ".", "--format sarif", 0, "valid", 1, expected_stderr=("managed by action inputs",)), + ScanScenario("config-passthrough-rejected", ".", "--config .keywatch.toml", 0, "valid", 1, expected_stderr=("managed by action inputs",)), ScanScenario("verbose-long-value-rejected", ".", "--verbose=true", 0, "valid", 1, expected_stderr=("managed by action inputs",)), ScanScenario("verbose-compact-short-rejected", ".", "-vv", 0, "valid", 1, expected_stderr=("managed by action inputs",)), ScanScenario("verbose-mode-long-allowed", ".", "--verbose-mode", 0, "valid", 0, expected_capture=("--verbose-mode",)), @@ -119,6 +122,7 @@ def run_scan_scenarios(scan_block: str) -> None: "INPUT_EXIT_MODE": "strict", "INPUT_OUTPUT": "", "INPUT_VERBOSE": "false", + "INPUT_CONFIG": scenario.config, "KEYWATCH_CAPTURE": str(workspace / "capture"), "KEYWATCH_STUB_EXIT": str(scenario.scanner_exit), "KEYWATCH_REPORT_MODE": scenario.report_mode, @@ -185,7 +189,8 @@ def main() -> int: require(fragment not in shell, f"forbidden shell fragment remains: {fragment}") require("${{ inputs." not in shell, "inputs must be routed through step env, not run blocks") - require("keywatch_args=(scan)" in shell, "scanner argv must be built with a Bash array") + require("keywatch_args=(scan --no-config-discovery)" in shell, "Action scans must disable untrusted config discovery") + require("keywatch_args+=(--config \"$INPUT_CONFIG\")" in shell, "explicit trusted config input must be supported") require("read -r -a paths" in shell, "paths input must be parsed without shell evaluation") require("compgen -G \"$path_token\"" in shell, "path globs must expand without shell evaluation") require("expanded_paths+=(\"$path_token\")" in shell, "unmatched globs and metachar literals must stay literal") @@ -194,6 +199,7 @@ def main() -> int: require("managed by action inputs" in shell, "args must not override verbose/output/exit-mode inputs") require("--verbose|--verbose=*|-v*" in shell, "all verbose arg forms must be rejected") require("--format|--format=*|-f|-f*" in shell, "all format arg forms must be rejected") + require("--config|--config=*|--no-config-discovery|--no-config-discovery=*" in shell, "config discovery controls must be managed by the Action") require("verbose output is disabled" in shell, "verbose mode must not log matched secrets") require("false|0|no|off|\"\")" in shell, "verbose=false must be an explicit non-verbose case") require("asset_arch=\"aarch64\"" in shell, "Darwin ARM64 must map to aarch64 release assets") diff --git a/src/baseline.rs b/src/baseline.rs index fbd7efe..502cf26 100644 --- a/src/baseline.rs +++ b/src/baseline.rs @@ -1,6 +1,11 @@ use serde::{Deserialize, Serialize}; use sha2::{Digest, Sha256}; -use std::{collections::HashSet, fs, path::Path}; +use std::{ + collections::HashSet, + error::Error, + fmt, fs, io, + path::{Path, PathBuf}, +}; use crate::report::Finding; @@ -67,6 +72,71 @@ pub struct Baseline { pub entries: Vec, } +#[derive(Debug)] +#[non_exhaustive] +pub enum BaselineError { + Read { + path: PathBuf, + source: io::Error, + }, + Parse { + path: PathBuf, + source: serde_json::Error, + }, + Serialize { + source: serde_json::Error, + }, + Write { + path: PathBuf, + source: io::Error, + }, +} + +impl fmt::Display for BaselineError { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::Read { path, source } => { + write!( + formatter, + "Failed to read baseline '{}': {}", + path.display(), + source + ) + } + Self::Parse { path, source } => { + write!( + formatter, + "Failed to parse baseline '{}': {}", + path.display(), + source + ) + } + Self::Serialize { source } => { + write!(formatter, "Failed to serialize baseline: {}", source) + } + Self::Write { path, source } => { + write!( + formatter, + "Failed to write baseline '{}': {}", + path.display(), + source + ) + } + } + } +} + +impl Error for BaselineError { + fn source(&self) -> Option<&(dyn Error + 'static)> { + match self { + Self::Read { source, .. } => Some(source), + Self::Parse { source, .. } => Some(source), + Self::Serialize { source } => Some(source), + Self::Write { source, .. } => Some(source), + } + } +} + impl Baseline { pub fn new() -> Self { Self { @@ -75,30 +145,37 @@ impl Baseline { } } - pub fn load(path: &Path) -> Result { + pub fn load(path: &Path) -> Result { if !path.exists() { return Ok(Self::new()); } - let contents = fs::read_to_string(path) - .map_err(|err| format!("Failed to read baseline '{}': {}", path.display(), err))?; + let contents = fs::read_to_string(path).map_err(|source| BaselineError::Read { + path: path.to_path_buf(), + source, + })?; if contents.trim().is_empty() { return Ok(Self::new()); } - let baseline: Baseline = serde_json::from_str(&contents) - .map_err(|err| format!("Failed to parse baseline '{}': {}", path.display(), err))?; + let baseline: Baseline = + serde_json::from_str(&contents).map_err(|source| BaselineError::Parse { + path: path.to_path_buf(), + source, + })?; Ok(baseline) } - pub fn save(&self, path: &Path) -> Result<(), String> { + pub fn save(&self, path: &Path) -> Result<(), BaselineError> { let json = serde_json::to_string_pretty(self) - .map_err(|err| format!("Failed to serialize baseline: {}", err))?; + .map_err(|source| BaselineError::Serialize { source })?; - fs::write(path, json) - .map_err(|err| format!("Failed to write baseline '{}': {}", path.display(), err))?; + fs::write(path, json).map_err(|source| BaselineError::Write { + path: path.to_path_buf(), + source, + })?; Ok(()) } diff --git a/src/cli.rs b/src/cli.rs index 007c9f4..c50550d 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -1,4 +1,42 @@ use clap::{Args, Parser, Subcommand, ValueEnum}; +use std::error::Error; +use std::fmt::{Display, Formatter}; + +#[derive(Debug, PartialEq, Eq)] +#[non_exhaustive] +pub enum CliValidationError { + GitHistoryWithStdin, + GitHistoryWithMultiplePaths, + StdinWithPaths, + MissingScanInput, + PreCommitRepositoryFilters, + PrePushExclude, +} + +impl Display for CliValidationError { + fn fmt(&self, formatter: &mut Formatter<'_>) -> std::fmt::Result { + match self { + Self::GitHistoryWithStdin => { + formatter.write_str("Cannot specify both --git-history and --stdin") + } + Self::GitHistoryWithMultiplePaths => { + formatter.write_str("Cannot specify more than one path with --git-history") + } + Self::StdinWithPaths => formatter.write_str("Cannot specify both --stdin and paths"), + Self::MissingScanInput => { + formatter.write_str("Must specify paths, use --stdin, or use --git-history") + } + Self::PreCommitRepositoryFilters => formatter.write_str( + "--allowed-repos and --blocked-repos are only supported for pre-push hooks", + ), + Self::PrePushExclude => { + formatter.write_str("--exclude is only supported for pre-commit hooks") + } + } + } +} + +impl Error for CliValidationError {} /// KeyWatch: A secret scanner for your files and directories. #[derive(Parser, Debug)] @@ -9,7 +47,7 @@ pub struct CliOptions { } impl CliOptions { - pub fn validate(&self) -> Result<(), String> { + pub fn validate(&self) -> Result<(), CliValidationError> { match &self.command { Command::Scan(args) => args.validate(), Command::Hook(args) => args.validate(), @@ -72,24 +110,36 @@ pub struct ScanArgs { /// Update the baseline file with current findings instead of scanning #[arg(long)] pub update_baseline: bool, + + /// Path to .keywatch.toml config file + #[arg(long)] + pub config: Option, + + /// Disable automatic config discovery (an explicit --config still loads) + #[arg(long, default_value_t = false)] + pub no_config_discovery: bool, + + /// Output format for the report (json or sarif) + #[arg(long, value_enum, default_value_t = OutputFormat::Json)] + pub format: OutputFormat, } impl ScanArgs { - pub fn validate(&self) -> Result<(), String> { + pub fn validate(&self) -> Result<(), CliValidationError> { if self.git_history { if self.stdin { - return Err("Cannot specify both --git-history and --stdin".to_string()); + return Err(CliValidationError::GitHistoryWithStdin); } if self.paths.len() > 1 { - return Err("Cannot specify more than one path with --git-history".to_string()); + return Err(CliValidationError::GitHistoryWithMultiplePaths); } return Ok(()); } if self.stdin && !self.paths.is_empty() { - return Err("Cannot specify both --stdin and paths".to_string()); + return Err(CliValidationError::StdinWithPaths); } if !self.stdin && self.paths.is_empty() { - return Err("Must specify paths, use --stdin, or use --git-history".to_string()); + return Err(CliValidationError::MissingScanInput); } Ok(()) } @@ -102,7 +152,7 @@ pub struct HookArgs { } impl HookArgs { - fn validate(&self) -> Result<(), String> { + fn validate(&self) -> Result<(), CliValidationError> { match &self.action { HookAction::Install(args) => args.validate(), HookAction::Uninstall(_) => Ok(()), @@ -143,19 +193,16 @@ pub struct HookInstallArgs { } impl HookInstallArgs { - fn validate(&self) -> Result<(), String> { + fn validate(&self) -> Result<(), CliValidationError> { match self.hook_type { HookType::PreCommit => { if self.allowed_repos.is_some() || self.blocked_repos.is_some() { - return Err( - "--allowed-repos and --blocked-repos are only supported for pre-push hooks" - .to_string(), - ); + return Err(CliValidationError::PreCommitRepositoryFilters); } } HookType::PrePush => { if self.exclude.is_some() { - return Err("--exclude is only supported for pre-commit hooks".to_string()); + return Err(CliValidationError::PrePushExclude); } } } @@ -216,6 +263,21 @@ pub enum ExitMode { Strict, } +#[derive(ValueEnum, Clone, Debug, PartialEq, Eq)] +pub enum OutputFormat { + Json, + Sarif, +} + +impl OutputFormat { + pub fn as_str(&self) -> &'static str { + match self { + Self::Json => "json", + Self::Sarif => "sarif", + } + } +} + impl ExitMode { pub fn as_str(&self) -> &'static str { match self { diff --git a/src/config/mod.rs b/src/config/mod.rs new file mode 100644 index 0000000..fea7c08 --- /dev/null +++ b/src/config/mod.rs @@ -0,0 +1,239 @@ +use crate::detector::Detector; +use crate::detector::DetectorError; +use crate::report::Severity; +use serde::Deserialize; +use std::collections::HashMap; +use std::error::Error as StdError; +use std::fmt; +use std::fs; +use std::path::{Path, PathBuf}; + +#[cfg(test)] +mod tests; + +/// User-facing configuration that extends and overrides the built-in detectors. +/// +/// Loaded from `--config ` or discovered from the first CLI scan path +/// (directory itself, or parent directory for a file path). Merges with +/// `detectors.toml`: custom rules are appended, overrides are applied by +/// detector name. +#[derive(Deserialize, Default)] +pub struct KeywatchConfig { + pub rules: Option>, + pub overrides: Option>, + pub exclude: Option>, +} + +#[derive(Debug)] +#[non_exhaustive] +pub enum ConfigError { + CurrentDirectory { + source: std::io::Error, + }, + NotFound { + path: String, + }, + Read { + path: String, + source: std::io::Error, + }, + Invalid { + source: toml::de::Error, + }, + CustomRule { + name: String, + source: DetectorError, + }, +} + +impl fmt::Display for ConfigError { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + ConfigError::CurrentDirectory { source } => { + write!( + formatter, + "Failed to determine current directory: {}", + source + ) + } + ConfigError::NotFound { path } => { + write!(formatter, "Config file not found: '{}'", path) + } + ConfigError::Read { path, source } => { + write!(formatter, "Failed to read config '{}': {}", path, source) + } + ConfigError::Invalid { source } => write!(formatter, "Invalid config: {}", source), + ConfigError::CustomRule { name, source } => { + write!(formatter, "custom rule '{}': {}", name, source) + } + } + } +} + +impl StdError for ConfigError { + fn source(&self) -> Option<&(dyn StdError + 'static)> { + match self { + ConfigError::CurrentDirectory { source } => Some(source), + ConfigError::NotFound { .. } => None, + ConfigError::Read { source, .. } => Some(source), + ConfigError::Invalid { source } => Some(source), + ConfigError::CustomRule { source, .. } => Some(source), + } + } +} + +impl KeywatchConfig { + /// Validate then apply this config to a detector set. + /// + /// All custom detectors are built in a scratch buffer first; on any error + /// the passed-in vector is left untouched (fail-closed / transactional). + pub fn apply_to(&self, detectors: &mut Vec) -> Result<(), ConfigError> { + // Phase 1 — validate: build every new detector before touching `detectors`. + let mut staged_detectors: Vec = Vec::new(); + if let Some(rules) = &self.rules { + for rule in rules { + let detector = Detector::new( + &rule.name, + &rule.pattern, + &rule.finding_type, + rule.severity.as_str(), + &[], + &[], + None, + ) + .map_err(|source| ConfigError::CustomRule { + name: rule.name.clone(), + source, + })?; + staged_detectors.push(detector); + } + } + + // Phase 2 — commit: all validation passed, mutate. + detectors.extend(staged_detectors); + + if let Some(overrides) = &self.overrides { + detectors.retain(|detector| match overrides.get(&detector.name) { + Some(detector_override) => detector_override.enabled != Some(false), + None => true, + }); + for detector in detectors.iter_mut() { + if let Some(severity) = overrides + .get(&detector.name) + .and_then(|detector_override| detector_override.severity) + { + detector.severity = severity; + } + } + } + + Ok(()) + } + + /// Load from an explicit path. When `path` is `None`, discovery starts in + /// the current working directory. + /// Returns `None` when no config file is found (config is optional). + pub fn load(path: Option<&str>) -> Result, ConfigError> { + let cwd = + std::env::current_dir().map_err(|source| ConfigError::CurrentDirectory { source })?; + Self::load_for_paths_at_cwd(path, &[], &cwd) + } + + /// Load config with scan-path-aware discovery. + /// + /// Resolution order: + /// 1. `explicit_path` — used as-is; returns `Err` if the file is absent. + /// 2. First CLI scan path that is a directory — search that directory. + /// 3. First CLI scan path that is a file — search its parent directory. + /// 4. Current working directory — only when `scan_paths` is empty. + /// + /// Candidate filenames tried in order: `.keywatch.toml`, `keywatch.toml`, + /// `.kw.toml`. + pub fn load_for_paths( + explicit_path: Option<&str>, + scan_paths: &[String], + ) -> Result, ConfigError> { + let cwd = + std::env::current_dir().map_err(|source| ConfigError::CurrentDirectory { source })?; + Self::load_for_paths_at_cwd(explicit_path, scan_paths, &cwd) + } + + fn load_for_paths_at_cwd( + explicit_path: Option<&str>, + scan_paths: &[String], + cwd: &Path, + ) -> Result, ConfigError> { + let config_path: Option = if let Some(path) = explicit_path { + if !Path::new(path).exists() { + return Err(ConfigError::NotFound { + path: path.to_string(), + }); + } + Some(path.to_string()) + } else { + find_config_candidates(scan_paths, cwd) + }; + + let config_path = match config_path { + Some(path) => path, + None => return Ok(None), + }; + + let contents = fs::read_to_string(&config_path).map_err(|source| ConfigError::Read { + path: config_path.clone(), + source, + })?; + + toml::from_str(&contents) + .map(Some) + .map_err(|source| ConfigError::Invalid { source }) + } +} + +#[derive(Deserialize, Clone)] +pub struct CustomRule { + pub name: String, + pub pattern: String, + pub finding_type: String, + #[serde(default = "default_severity")] + pub severity: Severity, + #[allow(dead_code)] + pub description: Option, +} + +#[derive(Deserialize, Clone)] +pub struct DetectorOverride { + pub enabled: Option, + pub severity: Option, +} + +fn default_severity() -> Severity { + Severity::Medium +} + +/// Candidate filenames tried in order when auto-discovering a config file. +const CONFIG_NAMES: [&str; 3] = [".keywatch.toml", "keywatch.toml", ".kw.toml"]; + +fn find_config_candidates(scan_paths: &[String], cwd: &Path) -> Option { + let search_dir: PathBuf = match scan_paths.first() { + Some(first) => { + let scan_path = Path::new(first); + if scan_path.is_dir() { + scan_path.to_path_buf() + } else { + scan_path + .parent() + .map(Path::to_path_buf) + .filter(|parent| parent != Path::new("")) + .unwrap_or_else(|| PathBuf::from(".")) + } + } + None => cwd.to_path_buf(), + }; + + CONFIG_NAMES + .iter() + .map(|name| search_dir.join(name)) + .find(|candidate_path| candidate_path.exists()) + .and_then(|candidate_path| candidate_path.to_str().map(str::to_string)) +} diff --git a/src/config/tests.rs b/src/config/tests.rs new file mode 100644 index 0000000..a659eaf --- /dev/null +++ b/src/config/tests.rs @@ -0,0 +1,23 @@ +mod application; +mod discovery; + +use crate::detector::Detector; +use std::io::Write; +use tempfile::TempDir; + +fn write_file(dir: &TempDir, name: &str, content: &str) -> std::path::PathBuf { + let path = dir.path().join(name); + let mut file = std::fs::File::create(&path).unwrap(); + file.write_all(content.as_bytes()).unwrap(); + path +} + +fn minimal_rule_toml(name: &str, pattern: &str, severity: &str) -> String { + format!( + "[[rules]]\nname = \"{name}\"\npattern = \"{pattern}\"\nfinding_type = \"Test\"\nseverity = \"{severity}\"\n" + ) +} + +fn make_detectors() -> Vec { + vec![Detector::new("ExistingDet", r"\bFOO\b", "Foo", "HIGH", &[], &[], None).unwrap()] +} diff --git a/src/config/tests/application.rs b/src/config/tests/application.rs new file mode 100644 index 0000000..c2752fb --- /dev/null +++ b/src/config/tests/application.rs @@ -0,0 +1,242 @@ +use super::make_detectors; +use crate::config::{ConfigError, DetectorOverride, KeywatchConfig}; +use crate::detector::DetectorError; +use crate::report::Severity; + +#[test] +fn test_severity_deserialize_case_insensitive() { + #[derive(serde::Deserialize)] + struct Wrap { + severity: Severity, + } + + for (input, expected) in [ + ("critical", Severity::Critical), + ("CRITICAL", Severity::Critical), + ("high", Severity::High), + ("HIGH", Severity::High), + ("medium", Severity::Medium), + ("MEDIUM", Severity::Medium), + ("low", Severity::Low), + ("LOW", Severity::Low), + ] { + let toml_str = format!("severity = \"{input}\""); + let wrap: Wrap = toml::from_str(&toml_str) + .unwrap_or_else(|err| panic!("failed to parse '{input}': {err}")); + assert_eq!(wrap.severity, expected, "input = {input}"); + } +} + +#[test] +fn test_severity_deserialize_invalid_aborts() { + #[derive(Debug, serde::Deserialize)] + struct Wrap { + #[allow(dead_code)] + severity: Severity, + } + + let result: Result = toml::from_str("severity = \"BOGUS\""); + assert!( + result.is_err(), + "invalid severity must fail deserialization" + ); + + let msg = result.unwrap_err().to_string(); + assert!( + msg.contains("BOGUS"), + "error should mention the bad value: {msg}" + ); +} + +#[test] +fn test_custom_rule_default_severity_is_medium() { + let toml_str = r#" +[[rules]] +name = "NoSev" +pattern = "\\bFOO\\b" +finding_type = "Test" +"#; + + let config: KeywatchConfig = toml::from_str(toml_str).unwrap(); + let rule = &config.rules.unwrap()[0]; + assert_eq!(rule.severity, Severity::Medium); +} + +#[test] +fn test_invalid_regex_returns_error() { + let toml_str = r#" +[[rules]] +name = "Bad" +pattern = "[invalid" +finding_type = "Test" +severity = "HIGH" +"#; + + let config: KeywatchConfig = toml::from_str(toml_str).unwrap(); + let mut detectors = make_detectors(); + let original_len = detectors.len(); + let result = config.apply_to(&mut detectors); + + let error = result.expect_err("invalid regex must return Err"); + match &error { + ConfigError::CustomRule { name, source } => { + assert_eq!(name, "Bad"); + assert!(matches!(source, &DetectorError::InvalidPattern { .. })); + } + other => panic!("expected CustomRule error, got {other:?}"), + } + let display = error.to_string(); + assert!( + display.starts_with("custom rule 'Bad': "), + "display prefix must be stable: {display}" + ); + assert_eq!( + detectors.len(), + original_len, + "detector vector must not be mutated on error" + ); +} + +#[test] +fn test_no_partial_mutation_second_rule_invalid() { + let toml_str = r#" +[[rules]] +name = "GoodRule" +pattern = "\\bGOOD\\b" +finding_type = "Good" +severity = "LOW" + +[[rules]] +name = "BadRule" +pattern = "[invalid" +finding_type = "Bad" +severity = "HIGH" +"#; + + let config: KeywatchConfig = toml::from_str(toml_str).unwrap(); + let mut detectors = make_detectors(); + let original_len = detectors.len(); + let result = config.apply_to(&mut detectors); + + assert!(matches!(result, Err(ConfigError::CustomRule { .. }))); + assert_eq!( + detectors.len(), + original_len, + "good rule must not be added when a later rule is invalid" + ); +} + +#[test] +fn test_invalid_custom_severity_aborts_at_parse() { + let toml_str = r#" +[[rules]] +name = "BadSev" +pattern = "\\bFOO\\b" +finding_type = "Test" +severity = "INVALID_SEVERITY" +"#; + + let result: Result = toml::from_str(toml_str); + assert!( + result.is_err(), + "invalid custom severity must fail TOML parsing" + ); +} + +#[test] +fn test_invalid_override_severity_aborts_at_parse() { + let toml_str = r#" +[overrides] +SomeDet = { severity = "NOT_A_SEVERITY" } +"#; + + let result: Result = toml::from_str(toml_str); + assert!( + result.is_err(), + "invalid override severity must fail TOML parsing" + ); +} + +#[test] +fn test_override_severity_applies_typed() { + let toml_str = r#" +[overrides] +ExistingDet = { severity = "LOW" } +"#; + + let config: KeywatchConfig = toml::from_str(toml_str).unwrap(); + let mut detectors = make_detectors(); + config.apply_to(&mut detectors).unwrap(); + assert_eq!(detectors[0].severity, Severity::Low); +} + +#[test] +fn test_override_disable() { + let toml_str = r#" +[overrides] +ExistingDet = { enabled = false } +"#; + + let config: KeywatchConfig = toml::from_str(toml_str).unwrap(); + let mut detectors = make_detectors(); + config.apply_to(&mut detectors).unwrap(); + assert!(detectors.is_empty(), "disabled detector must be removed"); +} + +#[test] +fn test_parse_minimal_config() { + let toml_str = r#" +[[rules]] +name = "TestDetector" +pattern = "\\bTEST_SECRET_[A-Z]+\\b" +finding_type = "Test Secret" +severity = "HIGH" +"#; + + let config: KeywatchConfig = toml::from_str(toml_str).expect("should parse"); + assert!(config.rules.is_some()); + assert_eq!(config.rules.as_ref().unwrap().len(), 1); + assert_eq!(config.rules.as_ref().unwrap()[0].name, "TestDetector"); + assert!(config.overrides.is_none()); +} + +#[test] +fn test_parse_config_with_overrides() { + let toml_str = r#" +[overrides] +AWSKeyDetector = { enabled = false } +GitHubTokenDetector = { severity = "CRITICAL" } +"#; + + let config: KeywatchConfig = toml::from_str(toml_str).expect("should parse"); + let overrides = config.overrides.expect("overrides present"); + assert_eq!(overrides.len(), 2); + + let aws = overrides.get("AWSKeyDetector").expect("found"); + assert_eq!(aws.enabled, Some(false)); + assert!(aws.severity.is_none()); + + let gh = overrides.get("GitHubTokenDetector").expect("found"); + assert_eq!(gh.severity, Some(Severity::Critical)); + assert!(gh.enabled.is_none()); +} + +#[test] +fn test_parse_override_severity_field_is_typed() { + let toml_str = r#" +[overrides] +Det = { severity = "medium" } +"#; + + let config: KeywatchConfig = toml::from_str(toml_str).unwrap(); + let ov: &DetectorOverride = config.overrides.as_ref().unwrap().get("Det").unwrap(); + assert_eq!(ov.severity, Some(Severity::Medium)); +} + +#[test] +fn test_empty_config_is_default() { + let config: KeywatchConfig = toml::from_str("").expect("empty should parse"); + assert!(config.rules.is_none()); + assert!(config.overrides.is_none()); + assert!(config.exclude.is_none()); +} diff --git a/src/config/tests/discovery.rs b/src/config/tests/discovery.rs new file mode 100644 index 0000000..44a4b38 --- /dev/null +++ b/src/config/tests/discovery.rs @@ -0,0 +1,179 @@ +use super::{minimal_rule_toml, write_file}; +use crate::config::{ConfigError, KeywatchConfig}; +use tempfile::TempDir; + +#[test] +fn test_config_discovery_directory_root() { + let dir = TempDir::new().unwrap(); + write_file( + &dir, + ".keywatch.toml", + &minimal_rule_toml("DirRule", r"\\bDIR\\b", "HIGH"), + ); + + let paths = vec![dir.path().to_str().unwrap().to_string()]; + let config = KeywatchConfig::load_for_paths(None, &paths) + .unwrap() + .expect("config should be found in directory"); + let names: Vec<_> = config + .rules + .unwrap() + .into_iter() + .map(|rule| rule.name) + .collect(); + assert!(names.contains(&"DirRule".to_string())); +} + +#[test] +fn test_config_discovery_file_parent() { + let dir = TempDir::new().unwrap(); + write_file( + &dir, + ".keywatch.toml", + &minimal_rule_toml("FileParentRule", r"\\bFP\\b", "MEDIUM"), + ); + let scan_file = write_file(&dir, "secrets.txt", "some content"); + + let paths = vec![scan_file.to_str().unwrap().to_string()]; + let config = KeywatchConfig::load_for_paths(None, &paths) + .unwrap() + .expect("config should be found in file's parent directory"); + let names: Vec<_> = config + .rules + .unwrap() + .into_iter() + .map(|rule| rule.name) + .collect(); + assert!(names.contains(&"FileParentRule".to_string())); +} + +#[test] +fn test_explicit_config_takes_precedence() { + let dir = TempDir::new().unwrap(); + write_file(&dir, ".keywatch.toml", "# empty"); + let explicit_dir = TempDir::new().unwrap(); + let explicit_cfg = write_file( + &explicit_dir, + "explicit.toml", + &minimal_rule_toml("ExplicitRule", r"\\bEXP\\b", "CRITICAL"), + ); + + let paths = vec![dir.path().to_str().unwrap().to_string()]; + let config = KeywatchConfig::load_for_paths(Some(explicit_cfg.to_str().unwrap()), &paths) + .unwrap() + .expect("explicit config must be loaded"); + let names: Vec<_> = config + .rules + .unwrap() + .into_iter() + .map(|rule| rule.name) + .collect(); + assert!(names.contains(&"ExplicitRule".to_string())); +} + +#[test] +fn test_no_paths_no_config_in_cwd_returns_ok() { + let result = KeywatchConfig::load_for_paths(None, &[]); + assert!(result.is_ok()); +} + +#[test] +fn test_explicit_nonexistent_path_returns_error() { + let result = KeywatchConfig::load_for_paths(Some("/nonexistent/path/config.toml"), &[]); + assert!(result.is_err()); + match result.err().unwrap() { + ConfigError::NotFound { path } => { + assert_eq!(path, "/nonexistent/path/config.toml"); + } + other => panic!("expected NotFound error, got {other:?}"), + } +} + +#[test] +fn test_candidate_filename_order_keywatch_toml_before_kw_toml() { + let dir = TempDir::new().unwrap(); + write_file( + &dir, + "keywatch.toml", + &minimal_rule_toml("KW", r"\\bKW\\b", "LOW"), + ); + write_file( + &dir, + ".kw.toml", + &minimal_rule_toml("KWShort", r"\\bKWS\\b", "LOW"), + ); + + let paths = vec![dir.path().to_str().unwrap().to_string()]; + let config = KeywatchConfig::load_for_paths(None, &paths) + .unwrap() + .expect("should find keywatch.toml"); + let names: Vec<_> = config + .rules + .unwrap() + .into_iter() + .map(|rule| rule.name) + .collect(); + assert!( + names.contains(&"KW".to_string()), + "keywatch.toml should win" + ); + assert!( + !names.contains(&"KWShort".to_string()), + ".kw.toml must not be picked" + ); +} + +#[test] +fn test_candidate_filename_dotted_wins_over_plain() { + let dir = TempDir::new().unwrap(); + write_file( + &dir, + ".keywatch.toml", + &minimal_rule_toml("Dotted", r"\\bDT\\b", "LOW"), + ); + write_file( + &dir, + "keywatch.toml", + &minimal_rule_toml("Plain", r"\\bPL\\b", "LOW"), + ); + + let paths = vec![dir.path().to_str().unwrap().to_string()]; + let config = KeywatchConfig::load_for_paths(None, &paths) + .unwrap() + .expect("should find .keywatch.toml"); + let names: Vec<_> = config + .rules + .unwrap() + .into_iter() + .map(|rule| rule.name) + .collect(); + assert!( + names.contains(&"Dotted".to_string()), + ".keywatch.toml should win" + ); + assert!( + !names.contains(&"Plain".to_string()), + "keywatch.toml must not be picked" + ); +} + +#[test] +fn test_load_none_uses_supplied_cwd() { + let dir = TempDir::new().unwrap(); + write_file( + &dir, + ".keywatch.toml", + &minimal_rule_toml("CwdRule", r"\\bCWD\\b", "LOW"), + ); + + let config = KeywatchConfig::load_for_paths_at_cwd(None, &[], dir.path()) + .unwrap() + .expect("config should be found in supplied cwd"); + let names: Vec<_> = config + .rules + .unwrap() + .into_iter() + .map(|rule| rule.name) + .collect(); + assert!(names.contains(&"CwdRule".to_string())); +} diff --git a/src/detector.rs b/src/detector.rs index e729813..0097cc4 100644 --- a/src/detector.rs +++ b/src/detector.rs @@ -1,3 +1,7 @@ +mod error; + +pub use error::DetectorInitError; + use crate::report::{ParseSeverityError, Severity}; use regex::Regex; use serde::Deserialize; @@ -20,26 +24,43 @@ pub enum DetectorError { } impl fmt::Display for DetectorError { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { match self { DetectorError::InvalidPattern { detector, source } => { - write!(f, "invalid pattern in detector '{}': {}", detector, source) + write!( + formatter, + "invalid pattern in detector '{}': {}", + detector, source + ) } DetectorError::InvalidAllowlistPattern { detector, source } => { write!( - f, + formatter, "invalid allowlist pattern in detector '{}': {}", detector, source ) } DetectorError::InvalidSeverity { detector, source } => { - write!(f, "invalid severity in detector '{}': {}", detector, source) + write!( + formatter, + "invalid severity in detector '{}': {}", + detector, source + ) } } } } -impl std::error::Error for DetectorError {} +impl std::error::Error for DetectorError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + match self { + Self::InvalidPattern { source, .. } | Self::InvalidAllowlistPattern { source, .. } => { + Some(source) + } + Self::InvalidSeverity { source, .. } => Some(source), + } + } +} pub struct Detector { pub name: String, @@ -73,9 +94,9 @@ impl Detector { })?; let mut compiled_allowlist = Vec::new(); - for pat in allowlist { + for pattern in allowlist { let compiled = - Regex::new(pat).map_err(|source| DetectorError::InvalidAllowlistPattern { + Regex::new(pattern).map_err(|source| DetectorError::InvalidAllowlistPattern { detector: name.to_string(), source, })?; @@ -97,10 +118,10 @@ impl Detector { if self.keywords.is_empty() { return true; } - let lower = content.to_lowercase(); + let lowercase_content = content.to_lowercase(); self.keywords .iter() - .any(|kw| lower.contains(&kw.to_lowercase())) + .any(|keyword| lowercase_content.contains(&keyword.to_lowercase())) } pub fn has_sufficient_entropy(&self, matched: &str) -> bool { @@ -111,18 +132,18 @@ impl Detector { } } -fn shannon_entropy(s: &str) -> f64 { - if s.is_empty() { +fn shannon_entropy(input: &str) -> f64 { + if input.is_empty() { return 0.0; } let mut counts = std::collections::HashMap::new(); - for ch in s.chars() { - *counts.entry(ch).or_insert(0) += 1; + for character in input.chars() { + *counts.entry(character).or_insert(0) += 1; } - let len = s.len() as f64; - counts.values().fold(0.0, |acc, &count| { - let p = count as f64 / len; - acc - p * p.log2() + let input_length = input.len() as f64; + counts.values().fold(0.0, |entropy, &count| { + let probability = count as f64 / input_length; + entropy - probability * probability.log2() }) } @@ -142,59 +163,83 @@ struct DetectorConfig { entropy: Option, } -fn find_detectors_config() -> std::path::PathBuf { +fn find_detectors_config(include_repository_config: bool) -> Option { std::env::var("KEYWATCH_CONFIG_PATH") .map(std::path::PathBuf::from) .ok() - .filter(|p| p.exists()) + .filter(|path| path.exists()) .or_else(|| { - let p = std::path::PathBuf::from("detectors.toml"); - if p.exists() { Some(p) } else { None } + if !include_repository_config { + return None; + } + + let repository_config = std::path::PathBuf::from("detectors.toml"); + repository_config.exists().then_some(repository_config) }) .or_else(|| { dirs::config_dir() - .map(|p| p.join("keywatch").join("detectors.toml")) - .filter(|p| p.exists()) + .map(|config_directory| config_directory.join("keywatch").join("detectors.toml")) + .filter(|path| path.exists()) }) .or_else(|| { std::env::current_exe() .ok() - .and_then(|p| p.parent().map(|d| d.join("detectors.toml"))) - .filter(|p| p.exists()) + .and_then(|executable_path| { + executable_path + .parent() + .map(|directory| directory.join("detectors.toml")) + }) + .filter(|path| path.exists()) }) - .unwrap_or_else(|| std::path::PathBuf::from("detectors.toml")) } -pub fn initialize_detectors() -> Result, Box> { - let config_path = find_detectors_config(); - let toml_contents = fs::read_to_string(&config_path) - .map_err(|err| format!("Failed to read {}: {}", config_path.display(), err))?; +pub fn initialize_detectors() -> Result, DetectorInitError> { + initialize_detectors_from_config(true) +} + +pub(crate) fn initialize_trusted_detectors() -> Result, DetectorInitError> { + initialize_detectors_from_config(false) +} + +fn initialize_detectors_from_config( + include_repository_config: bool, +) -> Result, DetectorInitError> { + let config_path = find_detectors_config(include_repository_config) + .ok_or(DetectorInitError::ConfigNotFound)?; + let toml_contents = + fs::read_to_string(&config_path).map_err(|source| DetectorInitError::ReadConfig { + path: config_path.clone(), + source, + })?; let config: DetectorsConfig = toml::from_str(&toml_contents) - .map_err(|err| format!("Failed to parse detectors.toml: {}", err))?; + .map_err(|source| DetectorInitError::ParseConfig { source })?; let mut seen_names = std::collections::HashSet::new(); - for det in &config.detectors { - if !seen_names.insert(det.name.as_str()) { - return Err(format!("duplicate detector name '{}'", det.name).into()); + for detector_config in &config.detectors { + if !seen_names.insert(detector_config.name.as_str()) { + return Err(DetectorInitError::DuplicateName { + detector: detector_config.name.clone(), + }); } } - Ok(config + config .detectors .into_iter() - .map(|det| { - let allowlist = det.allowlist.as_deref().unwrap_or_default(); - let keywords = det.keywords.as_deref().unwrap_or_default(); + .map(|detector_config| { + let allowlist = detector_config.allowlist.as_deref().unwrap_or_default(); + let keywords = detector_config.keywords.as_deref().unwrap_or_default(); Detector::new( - &det.name, - &det.pattern, - &det.finding_type, - &det.severity, + &detector_config.name, + &detector_config.pattern, + &detector_config.finding_type, + &detector_config.severity, allowlist, keywords, - det.entropy, + detector_config.entropy, ) }) - .collect::, _>>()?) + .collect::, _>>() + .map_err(|source| DetectorInitError::InvalidDetector { source }) } diff --git a/src/detector/error.rs b/src/detector/error.rs new file mode 100644 index 0000000..b936743 --- /dev/null +++ b/src/detector/error.rs @@ -0,0 +1,43 @@ +use super::DetectorError; +use std::{fmt, io, path::PathBuf}; + +#[derive(Debug)] +#[non_exhaustive] +pub enum DetectorInitError { + ConfigNotFound, + ReadConfig { path: PathBuf, source: io::Error }, + ParseConfig { source: toml::de::Error }, + DuplicateName { detector: String }, + InvalidDetector { source: DetectorError }, +} + +impl fmt::Display for DetectorInitError { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + DetectorInitError::ConfigNotFound => { + write!(formatter, "Failed to locate detectors.toml") + } + DetectorInitError::ReadConfig { path, source } => { + write!(formatter, "Failed to read {}: {}", path.display(), source) + } + DetectorInitError::ParseConfig { source } => { + write!(formatter, "Failed to parse detectors.toml: {}", source) + } + DetectorInitError::DuplicateName { detector } => { + write!(formatter, "duplicate detector name '{}'", detector) + } + DetectorInitError::InvalidDetector { source } => write!(formatter, "{}", source), + } + } +} + +impl std::error::Error for DetectorInitError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + match self { + DetectorInitError::ConfigNotFound | DetectorInitError::DuplicateName { .. } => None, + DetectorInitError::ReadConfig { source, .. } => Some(source), + DetectorInitError::ParseConfig { source } => Some(source), + DetectorInitError::InvalidDetector { source } => Some(source), + } + } +} diff --git a/src/hooks.rs b/src/hooks.rs index 72fafca..70708c8 100644 --- a/src/hooks.rs +++ b/src/hooks.rs @@ -1,10 +1,20 @@ -use crate::cli::HookInstallArgs; +use crate::cli::{HookInstallArgs, HookUninstallArgs}; +use crate::utils; +use std::env; +use std::fs; +use std::path::{Path, PathBuf}; +use std::process::Command as ProcessCommand; + +mod error; +pub use error::HookError; const PRE_PUSH_TEMPLATE: &str = include_str!("../templates/pre-push.sh"); const PRE_COMMIT_TEMPLATE: &str = include_str!("../templates/pre-commit.sh"); const DEFAULT_BINARY_NAME: &str = "key-watch"; +const KEYWATCH_MARKER: &str = "# Installed by KeyWatch"; + fn shell_escape(input: &str) -> String { format!("'{}'", input.replace('\'', "'\"'\"'")) } @@ -66,3 +76,519 @@ fn hook_binary_name() -> String { }) .unwrap_or_else(|| DEFAULT_BINARY_NAME.to_string()) } + +/// Install a git hook of the given type. +/// +/// Writes the rendered hook script to the resolved target path and makes it +/// executable. For global installs, a managed hooks directory is created and +/// configured via `git config --global core.hooksPath` if none is set. +pub fn install_hook(args: &HookInstallArgs) -> Result<(), HookError> { + let hook_content = match args.hook_type { + crate::cli::HookType::PrePush => generate_pre_push_hook(args), + crate::cli::HookType::PreCommit => generate_pre_commit_hook(args), + }; + let hook_type_str = args.hook_type.as_str(); + + let install_target = resolve_hook_install_target(hook_type_str, args.global)?; + + if !install_target.is_global { + ensure_local_hook_target_is_safe_to_create(&install_target.path)?; + } + + if let Some(parent) = install_target.path.parent() { + fs::create_dir_all(parent).map_err(|source| HookError::CreateHookDirectory { + path: parent.to_path_buf(), + source, + })?; + } + + if install_target.is_global { + ensure_global_hook_target_is_safe(&install_target.path)?; + } + + let hook_path = install_target.path.to_string_lossy().into_owned(); + utils::write_to_file(&hook_path, &hook_content).map_err(|source| HookError::InstallHook { + path: PathBuf::from(&hook_path), + source, + })?; + utils::make_executable(&hook_path).map_err(|source| HookError::MakeHookExecutable { + path: PathBuf::from(&hook_path), + source, + })?; + + if install_target.configured_global_path { + println!( + "Configured git --global core.hooksPath to {}", + install_target.hooks_dir.display() + ); + } + + if install_target.is_global { + println!("Installed global {hook_type_str} hook at {hook_path}"); + } else { + println!("Installed {hook_type_str} hook at {hook_path}"); + } + println!( + "The hook will run automatically during git {}.", + hook_type_str.replace('-', " ") + ); + + Ok(()) +} + +/// Uninstall a git hook of the given type. +/// +/// Only removes hooks that KeyWatch previously installed (verified via the +/// marker comment). Missing hooks are reported as a no-op success. +pub fn uninstall_hook(args: &HookUninstallArgs) -> Result<(), HookError> { + let hook_type_str = args.hook_type.as_str(); + let install_target = resolve_hook_uninstall_target(hook_type_str, args.global)?; + + if !install_target.path.exists() { + let scope = if install_target.is_global { + "global" + } else { + "local" + }; + println!( + "No {scope} {hook_type_str} hook found at {}", + install_target.path.display() + ); + return Ok(()); + } + + ensure_hook_target_is_keywatch_managed( + &install_target.path, + install_target.is_global, + "remove", + )?; + + fs::remove_file(&install_target.path).map_err(|source| HookError::RemoveHook { + path: install_target.path.clone(), + source, + })?; + + if install_target.is_global { + println!( + "Removed global {hook_type_str} hook at {}", + install_target.path.display() + ); + } else { + println!( + "Removed {hook_type_str} hook at {}", + install_target.path.display() + ); + } + + Ok(()) +} + +struct HookInstallTarget { + path: PathBuf, + hooks_dir: PathBuf, + is_global: bool, + configured_global_path: bool, +} + +impl HookInstallTarget { + fn local(hook_type: &'static str) -> Result { + let hooks_dir = resolve_local_hooks_dir()?; + Ok(Self { + path: hooks_dir.join(hook_type), + hooks_dir, + is_global: false, + configured_global_path: false, + }) + } + + fn global(hook_type: &'static str) -> Result { + if let Some(hooks_dir) = read_global_hooks_path()? { + return Ok(Self { + path: hooks_dir.join(hook_type), + hooks_dir, + is_global: true, + configured_global_path: false, + }); + } + + let managed_dir = managed_global_hooks_dir( + env::var_os("XDG_CONFIG_HOME"), + env::var_os("HOME"), + env::var_os("APPDATA"), + env::var_os("USERPROFILE"), + )?; + + fs::create_dir_all(&managed_dir).map_err(|source| { + HookError::CreateGlobalHooksDirectory { + path: managed_dir.clone(), + source, + } + })?; + configure_global_hooks_path(&managed_dir)?; + + Ok(Self { + path: managed_dir.join(hook_type), + hooks_dir: managed_dir, + is_global: true, + configured_global_path: true, + }) + } +} + +fn resolve_hook_install_target( + hook_type: &'static str, + global: bool, +) -> Result { + if global { + HookInstallTarget::global(hook_type) + } else { + HookInstallTarget::local(hook_type) + } +} + +fn resolve_hook_uninstall_target( + hook_type: &'static str, + global: bool, +) -> Result { + if !global { + return HookInstallTarget::local(hook_type); + } + + let hooks_dir = match read_global_hooks_path()? { + Some(path) => path, + None => managed_global_hooks_dir( + env::var_os("XDG_CONFIG_HOME"), + env::var_os("HOME"), + env::var_os("APPDATA"), + env::var_os("USERPROFILE"), + )?, + }; + + Ok(HookInstallTarget { + path: hooks_dir.join(hook_type), + hooks_dir, + is_global: true, + configured_global_path: false, + }) +} + +fn resolve_local_hooks_dir() -> Result { + resolve_local_hooks_dir_from(Path::new(".")) +} + +fn resolve_local_hooks_dir_from(cwd: &Path) -> Result { + let output = ProcessCommand::new("git") + .current_dir(cwd) + .args(["rev-parse", "--path-format=absolute", "--git-path", "hooks"]) + .output() + .map_err(|source| HookError::ResolveGitHooksDirectory { source })?; + + if !output.status.success() { + let stderr = String::from_utf8_lossy(&output.stderr).trim().to_string(); + return Err(if stderr.is_empty() { + HookError::MissingLocalRepository + } else { + HookError::ResolveGitHooksDirectoryFromGit { stderr } + }); + } + + let hooks_path = String::from_utf8_lossy(&output.stdout).trim().to_string(); + if hooks_path.is_empty() { + return Err(HookError::EmptyGitHooksDirectory); + } + + Ok(PathBuf::from(hooks_path)) +} + +fn read_global_hooks_path() -> Result, HookError> { + let output = ProcessCommand::new("git") + .args(["config", "--global", "--path", "--get", "core.hooksPath"]) + .output() + .map_err(|source| HookError::ReadGlobalHooksPath { source })?; + + if output.status.success() { + let value = String::from_utf8_lossy(&output.stdout).trim().to_string(); + return Ok(if value.is_empty() { + None + } else { + Some(PathBuf::from(value)) + }); + } + + if output.status.code() == Some(1) { + // Exit code 1 means the key is not set. + return Ok(None); + } + + let stderr = String::from_utf8_lossy(&output.stderr).trim().to_string(); + Err(HookError::ReadGlobalHooksPathFromGit { stderr }) +} + +fn managed_global_hooks_dir( + xdg_config_home: Option, + home: Option, + appdata: Option, + userprofile: Option, +) -> Result { + if let Some(xdg) = xdg_config_home { + return Ok(PathBuf::from(xdg).join("key-watch").join("hooks")); + } + if let Some(home) = home { + return Ok(PathBuf::from(home) + .join(".config") + .join("key-watch") + .join("hooks")); + } + if let Some(appdata) = appdata { + return Ok(PathBuf::from(appdata).join("key-watch").join("hooks")); + } + if let Some(userprofile) = userprofile { + return Ok(PathBuf::from(userprofile) + .join(".config") + .join("key-watch") + .join("hooks")); + } + Err(HookError::MissingGlobalHooksBaseDir) +} + +fn configure_global_hooks_path(hooks_dir: &Path) -> Result<(), HookError> { + let output = ProcessCommand::new("git") + .args(["config", "--global", "core.hooksPath"]) + .arg(hooks_dir) + .output() + .map_err(|source| HookError::ConfigureGlobalHooksPath { source })?; + + if output.status.success() { + Ok(()) + } else { + let stderr = String::from_utf8_lossy(&output.stderr).trim().to_string(); + Err(HookError::ConfigureGlobalHooksPathWithGit { + hooks_dir: hooks_dir.to_path_buf(), + stderr, + }) + } +} + +fn ensure_global_hook_target_is_safe(hook_path: &Path) -> Result<(), HookError> { + ensure_hook_target_is_keywatch_managed(hook_path, true, "overwrite") +} + +fn ensure_local_hook_target_is_safe_to_create(hook_path: &Path) -> Result<(), HookError> { + resolve_local_hooks_dir()?; + if hook_path.exists() { + ensure_hook_target_is_keywatch_managed(hook_path, false, "overwrite")?; + } + Ok(()) +} + +/// Refuse to touch an existing hook unless KeyWatch installed it. +fn ensure_hook_target_is_keywatch_managed( + hook_path: &Path, + is_global: bool, + action: &'static str, +) -> Result<(), HookError> { + if !hook_path.exists() { + return Ok(()); + } + + let content = + fs::read_to_string(hook_path).map_err(|source| HookError::InspectExistingHook { + path: hook_path.to_path_buf(), + source, + })?; + + if content.contains(KEYWATCH_MARKER) { + return Ok(()); + } + + let scope = if is_global { "global" } else { "local" }; + Err(HookError::RefuseExistingHook { + action, + scope, + path: hook_path.to_path_buf(), + }) +} + +#[cfg(test)] +mod tests { + use super::{ + HookError, ensure_global_hook_target_is_safe, ensure_local_hook_target_is_safe_to_create, + managed_global_hooks_dir, resolve_hook_uninstall_target, resolve_local_hooks_dir_from, + }; + use std::env; + use std::fs; + use std::process::Command; + use std::time::{SystemTime, UNIX_EPOCH}; + + fn unique_temp_dir(name: &str) -> std::path::PathBuf { + let timestamp = SystemTime::now() + .duration_since(UNIX_EPOCH) + .expect("System time should be after Unix epoch") + .as_millis(); + std::env::temp_dir().join(format!("keywatch_{name}_{timestamp}")) + } + + fn init_git_repo(path: &std::path::Path) { + fs::create_dir_all(path).expect("create repo dir"); + let status = Command::new("git") + .args(["init", "--quiet"]) + .current_dir(path) + .status() + .expect("run git init"); + assert!(status.success(), "git init should succeed"); + } + + #[test] + fn test_managed_global_hooks_dir_prefers_xdg() { + let path = managed_global_hooks_dir( + Some("/tmp/xdg".into()), + Some("/tmp/home".into()), + None, + None, + ) + .expect("xdg path should resolve"); + + assert_eq!(path, std::path::PathBuf::from("/tmp/xdg/key-watch/hooks")); + } + + #[test] + fn test_managed_global_hooks_dir_falls_back_to_home() { + let path = managed_global_hooks_dir(None, Some("/tmp/home".into()), None, None) + .expect("home path should resolve"); + + assert_eq!( + path, + std::path::PathBuf::from("/tmp/home/.config/key-watch/hooks") + ); + } + + #[test] + fn test_global_hook_safety_allows_keywatch_hook() { + let temp_dir = unique_temp_dir("global_hook_safe"); + fs::create_dir_all(&temp_dir).expect("create temp dir"); + let hook_path = temp_dir.join("pre-commit"); + + fs::write(&hook_path, "#!/bin/bash\n# Installed by KeyWatch\n").expect("write hook file"); + + ensure_global_hook_target_is_safe(&hook_path).expect("keywatch hook should be reusable"); + + fs::remove_file(&hook_path).expect("remove hook file"); + fs::remove_dir_all(&temp_dir).expect("remove temp dir"); + } + + #[test] + fn test_global_hook_safety_rejects_foreign_hook() { + let temp_dir = unique_temp_dir("global_hook_foreign"); + fs::create_dir_all(&temp_dir).expect("create temp dir"); + let hook_path = temp_dir.join("pre-commit"); + + fs::write(&hook_path, "#!/bin/bash\necho custom hook\n").expect("write hook file"); + + let error = ensure_global_hook_target_is_safe(&hook_path) + .expect_err("foreign hook should be rejected"); + assert!( + error + .to_string() + .contains("Refusing to overwrite existing global hook") + ); + + fs::remove_file(&hook_path).expect("remove hook file"); + fs::remove_dir_all(&temp_dir).expect("remove temp dir"); + } + + #[test] + fn test_resolve_local_hooks_dir_resolves_correctly() { + let temp_dir = unique_temp_dir("local_hooks_dir_resolution"); + init_git_repo(&temp_dir); + + let resolved = + resolve_local_hooks_dir_from(&temp_dir).expect("should resolve hooks dir inside repo"); + + assert!(resolved.is_absolute(), "hooks path should be absolute"); + assert!( + resolved.ends_with(".git/hooks"), + "hooks path should end with .git/hooks, but was: {}", + resolved.display() + ); + + fs::remove_dir_all(&temp_dir).expect("remove temp dir"); + } + + #[test] + fn test_git_init_creates_hooks_dir() { + let temp_dir = unique_temp_dir("local_hook_missing_git"); + init_git_repo(&temp_dir); + + assert!(temp_dir.join(".git/hooks").exists()); + + fs::remove_dir_all(&temp_dir).expect("remove temp dir"); + } + + #[test] + fn test_local_hook_safety_rejects_foreign_hook() { + let temp_dir = unique_temp_dir("local_hook_foreign"); + init_git_repo(&temp_dir); + let git_dir = temp_dir.join(".git"); + let git_hooks_dir = git_dir.join("hooks"); + fs::create_dir_all(&git_hooks_dir).expect("create hooks dir"); + let hook_path = git_hooks_dir.join("pre-commit"); + fs::write(&hook_path, "#!/bin/bash\necho custom hook\n").expect("write hook file"); + + let error = ensure_local_hook_target_is_safe_to_create(&hook_path) + .expect_err("foreign local hook should be rejected"); + assert!( + error + .to_string() + .contains("Refusing to overwrite existing local hook") + ); + + fs::remove_file(&hook_path).expect("remove hook file"); + fs::remove_dir_all(&temp_dir).expect("remove temp dir"); + } + + #[test] + fn test_global_uninstall_target_does_not_configure_missing_hooks_path() { + let install_target = resolve_hook_uninstall_target("pre-commit", true) + .expect("global uninstall target should resolve"); + let expected_hooks_dir = managed_global_hooks_dir( + env::var_os("XDG_CONFIG_HOME"), + env::var_os("HOME"), + env::var_os("APPDATA"), + env::var_os("USERPROFILE"), + ) + .expect("managed hooks dir should resolve"); + + assert!(install_target.is_global); + assert!(!install_target.configured_global_path); + assert_eq!(install_target.hooks_dir, expected_hooks_dir); + assert_eq!(install_target.path, expected_hooks_dir.join("pre-commit")); + } + + #[test] + fn test_managed_global_hooks_dir_prefers_appdata_when_home_missing() { + let path = managed_global_hooks_dir( + None, + None, + Some("C:/Users/test/AppData/Roaming".into()), + None, + ) + .expect("appdata path should resolve"); + + assert_eq!( + path, + std::path::PathBuf::from("C:/Users/test/AppData/Roaming/key-watch/hooks") + ); + } + + #[test] + fn test_managed_global_hooks_dir_errors_without_known_base_dir() { + let error = managed_global_hooks_dir(None, None, None, None) + .expect_err("missing env inputs should fail"); + + assert!(matches!(error, HookError::MissingGlobalHooksBaseDir)); + assert_eq!( + error.to_string(), + "Could not determine a directory for global git hooks" + ); + } +} diff --git a/src/hooks/error.rs b/src/hooks/error.rs new file mode 100644 index 0000000..0eb61fb --- /dev/null +++ b/src/hooks/error.rs @@ -0,0 +1,168 @@ +use std::error::Error; +use std::fmt::{Display, Formatter}; +use std::path::PathBuf; + +#[derive(Debug)] +#[non_exhaustive] +pub enum HookError { + CreateHookDirectory { + path: PathBuf, + source: std::io::Error, + }, + InstallHook { + path: PathBuf, + source: std::io::Error, + }, + MakeHookExecutable { + path: PathBuf, + source: std::io::Error, + }, + CreateGlobalHooksDirectory { + path: PathBuf, + source: std::io::Error, + }, + ResolveGitHooksDirectory { + source: std::io::Error, + }, + MissingLocalRepository, + ResolveGitHooksDirectoryFromGit { + stderr: String, + }, + EmptyGitHooksDirectory, + ReadGlobalHooksPath { + source: std::io::Error, + }, + ReadGlobalHooksPathFromGit { + stderr: String, + }, + MissingGlobalHooksBaseDir, + ConfigureGlobalHooksPath { + source: std::io::Error, + }, + ConfigureGlobalHooksPathWithGit { + hooks_dir: PathBuf, + stderr: String, + }, + InspectExistingHook { + path: PathBuf, + source: std::io::Error, + }, + RefuseExistingHook { + action: &'static str, + scope: &'static str, + path: PathBuf, + }, + RemoveHook { + path: PathBuf, + source: std::io::Error, + }, +} + +impl Display for HookError { + fn fmt(&self, formatter: &mut Formatter<'_>) -> std::fmt::Result { + match self { + Self::CreateHookDirectory { path, source } => write!( + formatter, + "Failed to create hook directory '{}': {}", + path.display(), + source + ), + Self::InstallHook { path, source } => write!( + formatter, + "Failed to install hook '{}': {}", + path.display(), + source + ), + Self::MakeHookExecutable { path, source } => write!( + formatter, + "Failed to make hook executable '{}': {}", + path.display(), + source + ), + Self::CreateGlobalHooksDirectory { path, source } => write!( + formatter, + "Failed to create global hooks directory '{}': {}", + path.display(), + source + ), + Self::ResolveGitHooksDirectory { source } => { + write!(formatter, "Failed to resolve git hooks directory: {source}") + } + Self::MissingLocalRepository => formatter + .write_str("Local hook installation requires running inside a git repository"), + Self::ResolveGitHooksDirectoryFromGit { stderr } => { + write!(formatter, "Failed to resolve git hooks directory: {stderr}") + } + Self::EmptyGitHooksDirectory => { + formatter.write_str("Failed to resolve git hooks directory") + } + Self::ReadGlobalHooksPath { source } => { + write!( + formatter, + "Failed to read git config core.hooksPath: {source}" + ) + } + Self::ReadGlobalHooksPathFromGit { stderr } => write!( + formatter, + "Failed to read git config core.hooksPath: {stderr}" + ), + Self::MissingGlobalHooksBaseDir => { + formatter.write_str("Could not determine a directory for global git hooks") + } + Self::ConfigureGlobalHooksPath { source } => write!( + formatter, + "Failed to configure git global core.hooksPath: {source}" + ), + Self::ConfigureGlobalHooksPathWithGit { hooks_dir, stderr } => write!( + formatter, + "git config --global core.hooksPath {} failed: {}", + hooks_dir.display(), + stderr + ), + Self::InspectExistingHook { path, source } => write!( + formatter, + "Failed to inspect existing hook '{}': {}", + path.display(), + source + ), + Self::RefuseExistingHook { + action, + scope, + path, + } => write!( + formatter, + "Refusing to {action} existing {scope} hook at '{}'. Merge it manually or remove it yourself.", + path.display() + ), + Self::RemoveHook { path, source } => write!( + formatter, + "Failed to remove hook '{}': {}", + path.display(), + source + ), + } + } +} + +impl Error for HookError { + fn source(&self) -> Option<&(dyn Error + 'static)> { + match self { + Self::CreateHookDirectory { source, .. } + | Self::InstallHook { source, .. } + | Self::MakeHookExecutable { source, .. } + | Self::CreateGlobalHooksDirectory { source, .. } + | Self::ResolveGitHooksDirectory { source } + | Self::ReadGlobalHooksPath { source } + | Self::ConfigureGlobalHooksPath { source } + | Self::InspectExistingHook { source, .. } + | Self::RemoveHook { source, .. } => Some(source), + Self::MissingLocalRepository + | Self::ResolveGitHooksDirectoryFromGit { .. } + | Self::EmptyGitHooksDirectory + | Self::ReadGlobalHooksPathFromGit { .. } + | Self::MissingGlobalHooksBaseDir + | Self::ConfigureGlobalHooksPathWithGit { .. } + | Self::RefuseExistingHook { .. } => None, + } + } +} diff --git a/src/lib.rs b/src/lib.rs index 75dd57f..e8644c9 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,37 +1,39 @@ pub mod baseline; pub mod cli; +pub mod config; pub mod detector; pub mod hooks; pub mod report; +mod run_error; pub mod scanner; pub mod utils; +pub use hooks::{generate_pre_commit_hook, generate_pre_push_hook}; +pub use run_error::RunCliError; + use clap::Parser; -use cli::{ - CliOptions, Command, ExitMode, HookAction, HookInstallArgs, HookType, HookUninstallArgs, - ScanArgs, Shell, -}; +use cli::{CliOptions, Command, ExitMode, HookAction, OutputFormat, ScanArgs, Shell}; use report::Finding; use std::env; -use std::fs; -use std::path::{Path, PathBuf}; -use std::process::Command as ProcessCommand; use std::time::Instant; use crate::report::Severity; -pub use hooks::{generate_pre_commit_hook, generate_pre_push_hook}; pub const EXIT_CODE_RUNTIME_ERROR: i32 = 2; -pub fn run_cli() -> Result<(), String> { +pub fn run_cli() -> Result<(), RunCliError> { let options = CliOptions::parse(); options.validate()?; match options.command { Command::Scan(args) => run_scan_command(&args), Command::Hook(args) => match args.action { - HookAction::Install(install_args) => install_hook(&install_args), - HookAction::Uninstall(uninstall_args) => uninstall_hook(&uninstall_args), + HookAction::Install(install_args) => { + hooks::install_hook(&install_args).map_err(Into::into) + } + HookAction::Uninstall(uninstall_args) => { + hooks::uninstall_hook(&uninstall_args).map_err(Into::into) + } }, Command::Init { shell } => { print_shell_init(&shell); @@ -41,10 +43,15 @@ pub fn run_cli() -> Result<(), String> { } } -fn run_scan_command(args: &ScanArgs) -> Result<(), String> { +fn run_scan_command(args: &ScanArgs) -> Result<(), RunCliError> { let start = Instant::now(); - let (mut findings, scan_metadata) = scanner::run_scan(args)?; + let config = if args.config.is_some() || !args.no_config_discovery { + config::KeywatchConfig::load_for_paths(args.config.as_deref(), &args.paths)? + } else { + None + }; + let (mut findings, scan_metadata) = scanner::run_scan(args, config.as_ref())?; if let Some(ref baseline_path) = args.baseline { let baseline = baseline::Baseline::load(std::path::Path::new(baseline_path))?; @@ -55,7 +62,7 @@ fn run_scan_command(args: &ScanArgs) -> Result<(), String> { let baseline_path = args .baseline .as_ref() - .ok_or("--update-baseline requires --baseline ")?; + .ok_or(RunCliError::MissingBaselineForUpdate)?; let mut baseline = baseline::Baseline::load(std::path::Path::new(baseline_path))?; baseline.update_with_findings(&findings); baseline.save(std::path::Path::new(baseline_path))?; @@ -72,11 +79,14 @@ fn run_scan_command(args: &ScanArgs) -> Result<(), String> { let severity_counts = report::get_severity_counts(&findings); let exit_code = calculate_exit_code(&findings, &args.exit_mode); let findings_count = findings.len(); - let report_json = report::create_report(findings, scan_metadata, scan_time) - .map_err(|err| format!("Failed to serialize report: {}", err))?; + let report_out = match args.format { + OutputFormat::Json => report::create_report(findings, scan_metadata, scan_time), + OutputFormat::Sarif => report::create_sarif_report(findings, scan_metadata, scan_time), + } + .map_err(|source| RunCliError::ReportSerialize { source })?; if args.verbose { - println!("{report_json}"); + println!("{report_out}"); } else if findings_count == 0 { println!("No secrets found."); } else { @@ -91,109 +101,15 @@ fn run_scan_command(args: &ScanArgs) -> Result<(), String> { } if let Some(ref output_path) = args.output { - utils::write_to_file(output_path, &report_json) - .map_err(|err| format!("Failed to write report to '{}': {}", output_path, err))?; - } - - std::process::exit(exit_code); -} - -fn install_hook(args: &HookInstallArgs) -> Result<(), String> { - let hook_content = match args.hook_type { - HookType::PrePush => hooks::generate_pre_push_hook(args), - HookType::PreCommit => hooks::generate_pre_commit_hook(args), - }; - - let hook_type_str = args.hook_type.as_str(); - let install_target = resolve_hook_install_target(hook_type_str, args.global)?; - - if !install_target.is_global { - ensure_local_hook_target_is_safe_to_create(&install_target.path)?; - } - - if let Some(parent) = install_target.path.parent() { - fs::create_dir_all(parent).map_err(|err| { - format!( - "Failed to create hook directory '{}': {}", - parent.display(), - err - ) + utils::write_to_file(output_path, &report_out).map_err(|source| { + RunCliError::ReportWrite { + path: output_path.clone(), + source, + } })?; } - if install_target.is_global { - ensure_global_hook_target_is_safe(&install_target.path)?; - } - - let hook_path = install_target.path.to_string_lossy().into_owned(); - utils::write_to_file(&hook_path, &hook_content) - .map_err(|err| format!("Failed to install hook '{}': {}", hook_type_str, err))?; - utils::make_executable(&hook_path) - .map_err(|err| format!("Failed to make hook executable '{}': {}", hook_path, err))?; - - if install_target.configured_global_path { - println!( - "Configured git --global core.hooksPath to {}", - install_target.hooks_dir.display() - ); - } - - if install_target.is_global { - println!("Installed global {hook_type_str} hook at {hook_path}"); - } else { - println!("Installed {hook_type_str} hook at {hook_path}"); - } - println!( - "The hook will run automatically during git {}.", - hook_type_str.replace('-', " ") - ); - Ok(()) -} - -fn uninstall_hook(args: &HookUninstallArgs) -> Result<(), String> { - let hook_type_str = args.hook_type.as_str(); - let install_target = resolve_hook_uninstall_target(hook_type_str, args.global)?; - - if !install_target.path.exists() { - let scope = if install_target.is_global { - "global" - } else { - "local" - }; - println!( - "No {scope} {hook_type_str} hook found at {}", - install_target.path.display() - ); - return Ok(()); - } - - ensure_hook_target_is_keywatch_managed( - &install_target.path, - install_target.is_global, - "remove", - )?; - - fs::remove_file(&install_target.path).map_err(|err| { - format!( - "Failed to remove hook '{}': {}", - install_target.path.display(), - err - ) - })?; - - if install_target.is_global { - println!( - "Removed global {hook_type_str} hook at {}", - install_target.path.display() - ); - } else { - println!( - "Removed {hook_type_str} hook at {}", - install_target.path.display() - ); - } - - Ok(()) + std::process::exit(exit_code); } fn print_shell_init(shell: &Shell) { @@ -207,255 +123,11 @@ fn print_shell_init(shell: &Shell) { print!("{script}"); } -struct HookInstallTarget { - path: PathBuf, - hooks_dir: PathBuf, - is_global: bool, - configured_global_path: bool, -} - -impl HookInstallTarget { - fn local(hook_type: &str) -> Result { - let hooks_dir = resolve_local_hooks_dir()?; - Ok(Self { - path: hooks_dir.join(hook_type), - hooks_dir, - is_global: false, - configured_global_path: false, - }) - } - - fn global(hook_type: &str) -> Result { - let configured = read_global_hooks_path()?; - let hooks_dir = match configured { - Some(path) => path, - None => { - let managed_dir = managed_global_hooks_dir( - env::var_os("XDG_CONFIG_HOME"), - env::var_os("HOME"), - env::var_os("APPDATA"), - env::var_os("USERPROFILE"), - )?; - fs::create_dir_all(&managed_dir).map_err(|err| { - format!( - "Failed to create global hooks directory '{}': {}", - managed_dir.display(), - err - ) - })?; - configure_global_hooks_path(&managed_dir)?; - return Ok(Self { - path: managed_dir.join(hook_type), - hooks_dir: managed_dir, - is_global: true, - configured_global_path: true, - }); - } - }; - - Ok(Self { - path: hooks_dir.join(hook_type), - hooks_dir, - is_global: true, - configured_global_path: false, - }) - } -} - -fn resolve_hook_install_target(hook_type: &str, global: bool) -> Result { - if global { - HookInstallTarget::global(hook_type) - } else { - HookInstallTarget::local(hook_type) - } -} - -fn resolve_hook_uninstall_target( - hook_type: &str, - global: bool, -) -> Result { - if global { - let hooks_dir = match read_global_hooks_path()? { - Some(path) => path, - None => managed_global_hooks_dir( - env::var_os("XDG_CONFIG_HOME"), - env::var_os("HOME"), - env::var_os("APPDATA"), - env::var_os("USERPROFILE"), - )?, - }; - - Ok(HookInstallTarget { - path: hooks_dir.join(hook_type), - hooks_dir, - is_global: true, - configured_global_path: false, - }) - } else { - HookInstallTarget::local(hook_type) - } -} - -fn resolve_local_hooks_dir() -> Result { - resolve_local_hooks_dir_from(Path::new(".")) -} - -fn resolve_local_hooks_dir_from(cwd: &Path) -> Result { - let output = ProcessCommand::new("git") - .current_dir(cwd) - .args(["rev-parse", "--path-format=absolute", "--git-path", "hooks"]) - .output() - .map_err(|err| format!("Failed to resolve git hooks directory: {}", err))?; - - if !output.status.success() { - let stderr = String::from_utf8_lossy(&output.stderr).trim().to_string(); - return Err(if stderr.is_empty() { - "Local hook installation requires running inside a git repository".to_string() - } else { - format!("Local hook installation requires running inside a git repository: {stderr}") - }); - } - - let hooks_path = String::from_utf8(output.stdout) - .map_err(|err| format!("Invalid UTF-8 from git rev-parse output: {}", err))? - .trim() - .to_string(); - - if hooks_path.is_empty() { - return Err("Failed to resolve git hooks directory".to_string()); - } - - Ok(PathBuf::from(hooks_path)) -} - -fn read_global_hooks_path() -> Result, String> { - let output = ProcessCommand::new("git") - .args(["config", "--global", "--path", "--get", "core.hooksPath"]) - .output() - .map_err(|err| format!("Failed to read git global core.hooksPath: {}", err))?; - - if output.status.success() { - let hooks_path = String::from_utf8(output.stdout) - .map_err(|err| format!("Invalid UTF-8 from git config output: {}", err))? - .trim() - .to_string(); - - if hooks_path.is_empty() { - Ok(None) - } else { - Ok(Some(PathBuf::from(hooks_path))) - } - } else if output.status.code() == Some(1) { - Ok(None) - } else { - Err(String::from_utf8_lossy(&output.stderr).trim().to_string()) - } -} - -fn managed_global_hooks_dir( - xdg_config_home: Option, - home: Option, - appdata: Option, - userprofile: Option, -) -> Result { - if let Some(xdg_config_home) = xdg_config_home { - return Ok(PathBuf::from(xdg_config_home) - .join("key-watch") - .join("hooks")); - } - - if let Some(home) = home { - return Ok(PathBuf::from(home) - .join(".config") - .join("key-watch") - .join("hooks")); - } - - if let Some(appdata) = appdata { - return Ok(PathBuf::from(appdata).join("key-watch").join("hooks")); - } - - if let Some(userprofile) = userprofile { - return Ok(PathBuf::from(userprofile) - .join(".config") - .join("key-watch") - .join("hooks")); - } - - Err("Could not determine a directory for global git hooks".to_string()) -} - -fn configure_global_hooks_path(hooks_dir: &Path) -> Result<(), String> { - let output = ProcessCommand::new("git") - .args([ - "config", - "--global", - "core.hooksPath", - hooks_dir.to_string_lossy().as_ref(), - ]) - .output() - .map_err(|err| format!("Failed to configure git global core.hooksPath: {}", err))?; - - if output.status.success() { - Ok(()) - } else { - let stderr = String::from_utf8_lossy(&output.stderr).trim().to_string(); - Err(format!( - "git config --global core.hooksPath {} failed: {}", - hooks_dir.display(), - stderr - )) - } -} - -fn ensure_global_hook_target_is_safe(hook_path: &Path) -> Result<(), String> { - ensure_hook_target_is_keywatch_managed(hook_path, true, "overwrite") -} - -fn ensure_local_hook_target_is_safe_to_create(hook_path: &Path) -> Result<(), String> { - resolve_local_hooks_dir()?; - - if hook_path.exists() { - ensure_hook_target_is_keywatch_managed(hook_path, false, "overwrite")?; - } - - Ok(()) -} - -fn ensure_hook_target_is_keywatch_managed( - hook_path: &Path, - is_global: bool, - action: &str, -) -> Result<(), String> { - if !hook_path.exists() { - return Ok(()); - } - - let existing_hook = fs::read_to_string(hook_path).map_err(|err| { - format!( - "Failed to inspect existing hook '{}': {}", - hook_path.display(), - err - ) - })?; - - if existing_hook.contains("# Installed by KeyWatch") { - return Ok(()); - } - - let scope = if is_global { "global" } else { "local" }; - Err(format!( - "Refusing to {action} existing {scope} hook at '{}'. Merge it manually or remove it yourself.", - hook_path.display() - )) -} - -fn verify_binary_integrity() -> Result<(), String> { - let exe_path = - env::current_exe().map_err(|err| format!("Failed to get executable path: {}", err))?; +fn verify_binary_integrity() -> Result<(), RunCliError> { + let exe_path = env::current_exe().map_err(|source| RunCliError::ExecutablePath { source })?; let metadata = exe_path .metadata() - .map_err(|err| format!("Failed to get executable metadata: {}", err))?; + .map_err(|source| RunCliError::ExecutableMetadata { source })?; #[cfg(unix)] { @@ -491,177 +163,9 @@ fn calculate_exit_code(findings: &[Finding], exit_mode: &ExitMode) -> i32 { #[cfg(test)] mod tests { - use super::{ - Severity, calculate_exit_code, ensure_global_hook_target_is_safe, - ensure_local_hook_target_is_safe_to_create, managed_global_hooks_dir, - resolve_hook_uninstall_target, resolve_local_hooks_dir_from, - }; + use super::{Severity, calculate_exit_code}; use crate::cli::ExitMode; use crate::report::Finding; - use std::env; - use std::fs; - use std::process::Command; - use std::time::{SystemTime, UNIX_EPOCH}; - - fn unique_temp_dir(name: &str) -> std::path::PathBuf { - let timestamp = SystemTime::now() - .duration_since(UNIX_EPOCH) - .expect("System time should be after Unix epoch") - .as_millis(); - std::env::temp_dir().join(format!("keywatch_{name}_{timestamp}")) - } - - fn init_git_repo(path: &std::path::Path) { - fs::create_dir_all(path).expect("create repo dir"); - let status = Command::new("git") - .args(["init", "--quiet"]) - .current_dir(path) - .status() - .expect("run git init"); - assert!(status.success(), "git init should succeed"); - } - - #[test] - fn test_managed_global_hooks_dir_prefers_xdg() { - let path = managed_global_hooks_dir( - Some("/tmp/xdg".into()), - Some("/tmp/home".into()), - None, - None, - ) - .expect("xdg path should resolve"); - - assert_eq!(path, std::path::PathBuf::from("/tmp/xdg/key-watch/hooks")); - } - - #[test] - fn test_managed_global_hooks_dir_falls_back_to_home() { - let path = managed_global_hooks_dir(None, Some("/tmp/home".into()), None, None) - .expect("home path should resolve"); - - assert_eq!( - path, - std::path::PathBuf::from("/tmp/home/.config/key-watch/hooks") - ); - } - - #[test] - fn test_global_hook_safety_allows_keywatch_hook() { - let temp_dir = unique_temp_dir("global_hook_safe"); - fs::create_dir_all(&temp_dir).expect("create temp dir"); - let hook_path = temp_dir.join("pre-commit"); - - fs::write(&hook_path, "#!/bin/bash\n# Installed by KeyWatch\n").expect("write hook file"); - - ensure_global_hook_target_is_safe(&hook_path).expect("keywatch hook should be reusable"); - - fs::remove_file(&hook_path).expect("remove hook file"); - fs::remove_dir_all(&temp_dir).expect("remove temp dir"); - } - - #[test] - fn test_global_hook_safety_rejects_foreign_hook() { - let temp_dir = unique_temp_dir("global_hook_foreign"); - fs::create_dir_all(&temp_dir).expect("create temp dir"); - let hook_path = temp_dir.join("pre-commit"); - - fs::write(&hook_path, "#!/bin/bash\necho custom hook\n").expect("write hook file"); - - let error = ensure_global_hook_target_is_safe(&hook_path) - .expect_err("foreign hook should be rejected"); - assert!(error.contains("Refusing to overwrite existing global hook")); - - fs::remove_file(&hook_path).expect("remove hook file"); - fs::remove_dir_all(&temp_dir).expect("remove temp dir"); - } - - #[test] - fn test_resolve_local_hooks_dir_resolves_correctly() { - let temp_dir = unique_temp_dir("local_hooks_dir_resolution"); - init_git_repo(&temp_dir); - - let resolved = - resolve_local_hooks_dir_from(&temp_dir).expect("should resolve hooks dir inside repo"); - - assert!(resolved.is_absolute(), "hooks path should be absolute"); - assert!( - resolved.ends_with(".git/hooks"), - "hooks path should end with .git/hooks, but was: {}", - resolved.display() - ); - - fs::remove_dir_all(&temp_dir).expect("remove temp dir"); - } - - #[test] - fn test_git_init_creates_hooks_dir() { - let temp_dir = unique_temp_dir("local_hook_missing_git"); - init_git_repo(&temp_dir); - - assert!(temp_dir.join(".git/hooks").exists()); - - fs::remove_dir_all(&temp_dir).expect("remove temp dir"); - } - - #[test] - fn test_local_hook_safety_rejects_foreign_hook() { - let temp_dir = unique_temp_dir("local_hook_foreign"); - init_git_repo(&temp_dir); - let git_dir = temp_dir.join(".git"); - let git_hooks_dir = git_dir.join("hooks"); - fs::create_dir_all(&git_hooks_dir).expect("create hooks dir"); - let hook_path = git_hooks_dir.join("pre-commit"); - fs::write(&hook_path, "#!/bin/bash\necho custom hook\n").expect("write hook file"); - - let error = ensure_local_hook_target_is_safe_to_create(&hook_path) - .expect_err("foreign local hook should be rejected"); - assert!(error.contains("Refusing to overwrite existing local hook")); - - fs::remove_file(&hook_path).expect("remove hook file"); - fs::remove_dir_all(&temp_dir).expect("remove temp dir"); - } - - #[test] - fn test_global_uninstall_target_does_not_configure_missing_hooks_path() { - let install_target = resolve_hook_uninstall_target("pre-commit", true) - .expect("global uninstall target should resolve"); - let expected_hooks_dir = managed_global_hooks_dir( - env::var_os("XDG_CONFIG_HOME"), - env::var_os("HOME"), - env::var_os("APPDATA"), - env::var_os("USERPROFILE"), - ) - .expect("managed hooks dir should resolve"); - - assert!(install_target.is_global); - assert!(!install_target.configured_global_path); - assert_eq!(install_target.hooks_dir, expected_hooks_dir); - assert_eq!(install_target.path, expected_hooks_dir.join("pre-commit")); - } - - #[test] - fn test_managed_global_hooks_dir_prefers_appdata_when_home_missing() { - let path = managed_global_hooks_dir( - None, - None, - Some("C:/Users/test/AppData/Roaming".into()), - None, - ) - .expect("appdata path should resolve"); - - assert_eq!( - path, - std::path::PathBuf::from("C:/Users/test/AppData/Roaming/key-watch/hooks") - ); - } - - #[test] - fn test_managed_global_hooks_dir_errors_without_known_base_dir() { - let error = managed_global_hooks_dir(None, None, None, None) - .expect_err("missing env inputs should fail"); - - assert!(error.contains("Could not determine a directory for global git hooks")); - } #[test] fn test_calculate_exit_code_across_modes() { diff --git a/src/report.rs b/src/report.rs index adff5a9..c912167 100644 --- a/src/report.rs +++ b/src/report.rs @@ -1,5 +1,9 @@ -use serde::Serialize; +use serde::de::{self, Visitor}; +use serde::{Deserialize, Deserializer, Serialize}; use std::{fmt, str::FromStr}; +mod sarif; + +pub use sarif::create_sarif_report; #[derive(Serialize, Clone, PartialEq, Copy, Debug)] #[serde(rename_all = "UPPERCASE")] @@ -10,6 +14,18 @@ pub enum Severity { Low, } +impl Severity { + /// Return the canonical uppercase string representation. + pub fn as_str(self) -> &'static str { + match self { + Severity::Critical => "CRITICAL", + Severity::High => "HIGH", + Severity::Medium => "MEDIUM", + Severity::Low => "LOW", + } + } +} + /// Error returned when a string cannot be parsed as a [`Severity`]. #[derive(Debug, PartialEq)] pub struct ParseSeverityError { @@ -17,9 +33,9 @@ pub struct ParseSeverityError { } impl fmt::Display for ParseSeverityError { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { write!( - f, + formatter, "invalid severity '{}': expected one of CRITICAL, HIGH, MEDIUM, LOW", self.input ) @@ -44,21 +60,35 @@ impl FromStr for Severity { } } -impl Severity { - /// Return the canonical uppercase string representation. - pub fn as_str(self) -> &'static str { - match self { - Severity::Critical => "CRITICAL", - Severity::High => "HIGH", - Severity::Medium => "MEDIUM", - Severity::Low => "LOW", +/// Deserialize [`Severity`] from a string using the existing case-insensitive +/// [`FromStr`] implementation. Serialization output (`UPPERCASE`) is governed +/// by the `#[serde(rename_all = "UPPERCASE")]` derive above and is not +/// affected by this impl. +impl<'de> Deserialize<'de> for Severity { + fn deserialize>( + deserializer: DeserializerType, + ) -> Result { + struct SeverityVisitor; + + impl Visitor<'_> for SeverityVisitor { + type Value = Severity; + + fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + formatter.write_str("one of CRITICAL, HIGH, MEDIUM, LOW (case-insensitive)") + } + + fn visit_str(self, value: &str) -> Result { + Severity::from_str(value).map_err(de::Error::custom) + } } + + deserializer.deserialize_str(SeverityVisitor) } } impl fmt::Display for Severity { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.write_str(self.as_str()) + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + formatter.write_str(self.as_str()) } } diff --git a/src/report/sarif.rs b/src/report/sarif.rs new file mode 100644 index 0000000..bb92689 --- /dev/null +++ b/src/report/sarif.rs @@ -0,0 +1,158 @@ +use super::{Finding, ScanMetadata, Severity}; +use serde::Serialize; +use std::collections::HashMap; + +/// Generate a SARIF 2.1.0 report from findings. +pub fn create_sarif_report( + findings: Vec, + _metadata: ScanMetadata, + scan_time: String, +) -> Result { + #[derive(Serialize)] + #[serde(rename_all = "camelCase")] + struct SarifLog { + #[serde(rename = "$schema")] + schema: &'static str, + version: &'static str, + runs: Vec, + } + + #[derive(Serialize)] + #[serde(rename_all = "camelCase")] + struct SarifRun { + tool: SarifTool, + results: Vec, + properties: HashMap, + } + + #[derive(Serialize)] + #[serde(rename_all = "camelCase")] + struct SarifTool { + driver: SarifDriver, + } + + #[derive(Serialize)] + #[serde(rename_all = "camelCase")] + struct SarifDriver { + name: &'static str, + #[serde(skip_serializing_if = "Option::is_none")] + version: Option, + information_uri: &'static str, + semantic_version: Option, + } + + #[derive(Serialize)] + #[serde(rename_all = "camelCase")] + struct SarifResult { + rule_id: String, + level: &'static str, + message: SarifMessage, + locations: Vec, + properties: HashMap, + } + + #[derive(Serialize)] + #[serde(rename_all = "camelCase")] + struct SarifMessage { + text: String, + } + + #[derive(Serialize)] + #[serde(rename_all = "camelCase")] + struct SarifLocation { + physical_location: SarifPhysicalLocation, + } + + #[derive(Serialize)] + #[serde(rename_all = "camelCase")] + struct SarifPhysicalLocation { + artifact_location: SarifArtifactLocation, + region: SarifRegion, + } + + #[derive(Serialize)] + #[serde(rename_all = "camelCase")] + struct SarifArtifactLocation { + uri: String, + } + + #[derive(Serialize)] + #[serde(rename_all = "camelCase")] + struct SarifRegion { + start_line: usize, + } + + fn severity_to_sarif_level(severity: Severity) -> &'static str { + match severity { + Severity::Critical | Severity::High => "error", + Severity::Medium => "warning", + Severity::Low => "note", + } + } + + let results: Vec = findings + .into_iter() + .map(|finding| { + let rule_id = finding.finding_type; + let level = severity_to_sarif_level(finding.severity); + let rule_id_clone = rule_id.clone(); + let severity_str = finding.severity.as_str(); + let uri = finding.file_path; + let start_line = finding.line_number; + + let mut properties = HashMap::new(); + properties.insert( + "precision".to_string(), + serde_json::Value::String("very-high".to_string()), + ); + properties.insert( + "severity".to_string(), + serde_json::Value::String(severity_str.to_string()), + ); + + SarifResult { + rule_id, + level, + message: SarifMessage { + text: format!("Potential {} detected", rule_id_clone), + }, + locations: vec![SarifLocation { + physical_location: SarifPhysicalLocation { + artifact_location: SarifArtifactLocation { uri }, + region: SarifRegion { start_line }, + }, + }], + properties, + } + }) + .collect(); + + let status = if results.is_empty() { "pass" } else { "fail" }; + + let mut properties = HashMap::new(); + properties.insert( + "status".to_string(), + serde_json::Value::String(status.to_string()), + ); + properties.insert("scanTime".to_string(), serde_json::Value::String(scan_time)); + + let log = SarifLog { + schema: "https://raw.githubusercontent.com/oasis-tcs/sarif-spec/master/Schemata/sarif-schema-2.1.0.json", + version: "2.1.0", + runs: vec![SarifRun { + tool: SarifTool { + driver: SarifDriver { + name: "KeyWatch", + version: option_env!("CARGO_PKG_VERSION").map(|version| version.to_string()), + information_uri: "https://github.com/pixincreate/KeyWatch", + semantic_version: option_env!("CARGO_PKG_VERSION") + .map(|version| version.to_string()), + }, + }, + results, + properties, + }], + }; + + serde_json::to_string_pretty(&log) +} diff --git a/src/run_error.rs b/src/run_error.rs new file mode 100644 index 0000000..f77fce3 --- /dev/null +++ b/src/run_error.rs @@ -0,0 +1,98 @@ +use std::error::Error as StdError; +use std::fmt::{self, Display, Formatter}; +use std::io; + +use crate::baseline::BaselineError; +use crate::cli::CliValidationError; +use crate::config::ConfigError; +use crate::hooks::HookError; +use crate::scanner::ScannerError; + +#[derive(Debug)] +#[non_exhaustive] +pub enum RunCliError { + Cli { source: CliValidationError }, + Config { source: ConfigError }, + Scanner { source: ScannerError }, + Baseline { source: BaselineError }, + Hooks { source: HookError }, + MissingBaselineForUpdate, + ReportSerialize { source: serde_json::Error }, + ReportWrite { path: String, source: io::Error }, + ExecutablePath { source: io::Error }, + ExecutableMetadata { source: io::Error }, +} + +impl Display for RunCliError { + fn fmt(&self, formatter: &mut Formatter<'_>) -> fmt::Result { + match self { + Self::Cli { source } => write!(formatter, "{source}"), + Self::Config { source } => write!(formatter, "{source}"), + Self::Scanner { source } => write!(formatter, "{source}"), + Self::Baseline { source } => write!(formatter, "{source}"), + Self::Hooks { source } => write!(formatter, "{source}"), + Self::MissingBaselineForUpdate => { + formatter.write_str("--update-baseline requires --baseline ") + } + Self::ReportSerialize { source } => { + write!(formatter, "Failed to serialize report: {source}") + } + Self::ReportWrite { path, source } => { + write!(formatter, "Failed to write report to '{path}': {source}") + } + Self::ExecutablePath { source } => { + write!(formatter, "Failed to get executable path: {source}") + } + Self::ExecutableMetadata { source } => { + write!(formatter, "Failed to get executable metadata: {source}") + } + } + } +} + +impl StdError for RunCliError { + fn source(&self) -> Option<&(dyn StdError + 'static)> { + match self { + Self::Cli { source } => Some(source), + Self::Config { source } => Some(source), + Self::Scanner { source } => Some(source), + Self::Baseline { source } => Some(source), + Self::Hooks { source } => Some(source), + Self::MissingBaselineForUpdate => None, + Self::ReportSerialize { source } => Some(source), + Self::ReportWrite { source, .. } => Some(source), + Self::ExecutablePath { source } => Some(source), + Self::ExecutableMetadata { source } => Some(source), + } + } +} + +impl From for RunCliError { + fn from(source: CliValidationError) -> Self { + Self::Cli { source } + } +} + +impl From for RunCliError { + fn from(source: ConfigError) -> Self { + Self::Config { source } + } +} + +impl From for RunCliError { + fn from(source: ScannerError) -> Self { + Self::Scanner { source } + } +} + +impl From for RunCliError { + fn from(source: BaselineError) -> Self { + Self::Baseline { source } + } +} + +impl From for RunCliError { + fn from(source: HookError) -> Self { + Self::Hooks { source } + } +} diff --git a/src/scanner.rs b/src/scanner.rs index e7840d6..f790535 100644 --- a/src/scanner.rs +++ b/src/scanner.rs @@ -1,5 +1,6 @@ use crate::cli::ScanArgs; -use crate::detector::{Detector, initialize_detectors}; +use crate::config::KeywatchConfig; +use crate::detector::{Detector, initialize_detectors, initialize_trusted_detectors}; use crate::report::{Finding, ScanMetadata}; use glob::Pattern; use rayon::prelude::*; @@ -7,6 +8,10 @@ use std::fs; use std::io::{BufRead, BufReader}; use std::path::Path; +mod error; + +pub use error::ScannerError; + const INLINE_SUPPRESS: &str = "keywatch:ignore"; fn is_inline_suppressed(line: &str) -> bool { @@ -113,12 +118,12 @@ fn scan_content( (findings, total_lines) } -fn scan_stream( - reader: R, +fn scan_stream( + reader: ReaderType, path: &str, multiline_detectors: &[&Detector], line_detectors: &[&Detector], -) -> Result<(Vec, usize), String> { +) -> Result<(Vec, usize), ScannerError> { const CHUNK_SIZE: usize = 1000; const OVERLAP_LINES: usize = 50; @@ -128,7 +133,10 @@ fn scan_stream( let mut line_offset = 0; for line_result in reader.lines() { - let line = line_result.map_err(|e| format!("Read error on {}: {}", path, e))?; + let line = line_result.map_err(|source| ScannerError::ReadStream { + path: path.to_string(), + source, + })?; total_lines += 1; scan_line_detectors( @@ -170,8 +178,21 @@ fn scan_stream( Ok((findings, total_lines)) } -pub fn run_scan(args: &ScanArgs) -> Result<(Vec, ScanMetadata), String> { - let detectors = initialize_detectors().map_err(|err| err.to_string())?; +pub fn run_scan( + args: &ScanArgs, + config: Option<&KeywatchConfig>, +) -> Result<(Vec, ScanMetadata), ScannerError> { + let mut detectors = if args.no_config_discovery { + initialize_trusted_detectors() + } else { + initialize_detectors() + } + .map_err(|source| ScannerError::DetectorInit { source })?; + + if let Some(cfg) = config { + cfg.apply_to(&mut detectors) + .map_err(|source| ScannerError::Config { source })?; + } let (multiline_detectors, line_detectors): (Vec<_>, Vec<_>) = detectors .iter() .partition(|detector| detector.regex.as_str().contains("(?s)")); @@ -195,9 +216,9 @@ pub fn run_scan(args: &ScanArgs) -> Result<(Vec, ScanMetadata), String> ]) .stdout(std::process::Stdio::piped()) .spawn() - .map_err(|err| format!("Failed to run git log: {}", err))?; + .map_err(|source| ScannerError::RunGitLog { source })?; - let stdout = child.stdout.take().ok_or("Failed to capture git stdout")?; + let stdout = child.stdout.take().ok_or(ScannerError::CaptureGitStdout)?; let reader = BufReader::new(stdout); let (findings, total_lines) = scan_stream( reader, @@ -208,9 +229,9 @@ pub fn run_scan(args: &ScanArgs) -> Result<(Vec, ScanMetadata), String> let status = child .wait() - .map_err(|e| format!("git process error: {}", e))?; + .map_err(|source| ScannerError::GitProcess { source })?; if !status.success() { - return Err("git log exited with non-zero status".to_string()); + return Err(ScannerError::GitLogNonZero); } let metadata = ScanMetadata { @@ -267,7 +288,7 @@ pub fn run_scan(args: &ScanArgs) -> Result<(Vec, ScanMetadata), String> } let unique_paths: Vec<_> = unique_paths.into_iter().collect(); - let exclude_patterns: Vec = args + let mut exclude_patterns: Vec = args .exclude .as_ref() .map(|exclude_str| { @@ -275,14 +296,29 @@ pub fn run_scan(args: &ScanArgs) -> Result<(Vec, ScanMetadata), String> .split(',') .filter(|pattern| !pattern.trim().is_empty()) .map(|pattern| { - Pattern::new(pattern.trim()) - .map_err(|err| format!("Invalid exclude pattern '{}': {}", pattern, err)) + Pattern::new(pattern.trim()).map_err(|source| { + ScannerError::InvalidExcludePattern { + pattern: pattern.to_string(), + source, + } + }) }) .collect::, _>>() }) .transpose()? .unwrap_or_default(); + if let Some(excludes) = config.and_then(|cfg| cfg.exclude.as_ref()) { + for pattern_str in excludes { + exclude_patterns.push(Pattern::new(pattern_str).map_err(|source| { + ScannerError::InvalidConfigExcludePattern { + pattern: pattern_str.to_string(), + source, + } + })?); + } + } + let results: Vec<(Vec, usize, usize, Option)> = unique_paths .into_par_iter() .map(|(path, roots)| { @@ -408,8 +444,8 @@ mod tests { use crate::detector::Detector; use std::io::Cursor; - fn make_detector(name: &str, pattern: &str, ftype: &str, sev: &str) -> Detector { - Detector::new(name, pattern, ftype, sev, &[], &[], None).unwrap() + fn make_detector(name: &str, pattern: &str, finding_type: &str, severity: &str) -> Detector { + Detector::new(name, pattern, finding_type, severity, &[], &[], None).unwrap() } #[test] @@ -425,16 +461,25 @@ mod tests { "HIGH", ), ]; - let (multi, line): (Vec<_>, Vec<_>) = detectors + let (multiline_detectors, line_detectors): (Vec<_>, Vec<_>) = detectors .iter() - .partition(|d| d.regex.as_str().contains("(?s)")); + .partition(|detector| detector.regex.as_str().contains("(?s)")); - let (findings, lines) = scan_stream(reader, "", &multi, &line).unwrap(); + let (findings, total_lines) = + scan_stream(reader, "", &multiline_detectors, &line_detectors).unwrap(); - assert_eq!(lines, 2); + assert_eq!(total_lines, 2); assert_eq!(findings.len(), 2); - assert!(findings.iter().any(|f| f.finding_type == "AWS Key")); - assert!(findings.iter().any(|f| f.finding_type == "Password")); + assert!( + findings + .iter() + .any(|finding| finding.finding_type == "AWS Key") + ); + assert!( + findings + .iter() + .any(|finding| finding.finding_type == "Password") + ); } #[test] @@ -447,11 +492,12 @@ mod tests { "Password", "HIGH", )]; - let (multi, line): (Vec<_>, Vec<_>) = detectors + let (multiline_detectors, line_detectors): (Vec<_>, Vec<_>) = detectors .iter() - .partition(|d| d.regex.as_str().contains("(?s)")); + .partition(|detector| detector.regex.as_str().contains("(?s)")); - let (findings, _) = scan_stream(reader, "", &multi, &line).unwrap(); + let (findings, _) = + scan_stream(reader, "", &multiline_detectors, &line_detectors).unwrap(); assert!( findings.is_empty(), "Suppressed line should produce no findings" @@ -468,11 +514,12 @@ mod tests { "Private Key", "HIGH", )]; - let (multi, line): (Vec<_>, Vec<_>) = detectors + let (multiline_detectors, line_detectors): (Vec<_>, Vec<_>) = detectors .iter() - .partition(|d| d.regex.as_str().contains("(?s)")); + .partition(|detector| detector.regex.as_str().contains("(?s)")); - let (findings, _) = scan_stream(reader, "", &multi, &line).unwrap(); + let (findings, _) = + scan_stream(reader, "", &multiline_detectors, &line_detectors).unwrap(); assert_eq!(findings.len(), 1); assert_eq!(findings[0].finding_type, "Private Key"); } @@ -480,8 +527,11 @@ mod tests { #[test] fn test_scan_stream_large_input_chunked() { let mut content = String::new(); - for i in 0..2500 { - content.push_str(&format!("line {}: password = 'secret{}'\n", i, i)); + for line_number in 0..2500 { + content.push_str(&format!( + "line {}: password = 'secret{}'\n", + line_number, line_number + )); } let reader = Cursor::new(content); let detectors = [make_detector( @@ -490,12 +540,13 @@ mod tests { "Password", "HIGH", )]; - let (multi, line): (Vec<_>, Vec<_>) = detectors + let (multiline_detectors, line_detectors): (Vec<_>, Vec<_>) = detectors .iter() - .partition(|d| d.regex.as_str().contains("(?s)")); + .partition(|detector| detector.regex.as_str().contains("(?s)")); - let (findings, lines) = scan_stream(reader, "", &multi, &line).unwrap(); - assert_eq!(lines, 2500); + let (findings, total_lines) = + scan_stream(reader, "", &multiline_detectors, &line_detectors).unwrap(); + assert_eq!(total_lines, 2500); assert_eq!( findings.len(), 2500, diff --git a/src/scanner/error.rs b/src/scanner/error.rs new file mode 100644 index 0000000..9c73fe9 --- /dev/null +++ b/src/scanner/error.rs @@ -0,0 +1,82 @@ +use crate::config::ConfigError; +use crate::detector::DetectorInitError; +use std::error::Error as StdError; +use std::{fmt, io}; + +#[derive(Debug)] +#[non_exhaustive] +pub enum ScannerError { + DetectorInit { + source: DetectorInitError, + }, + Config { + source: ConfigError, + }, + ReadStream { + path: String, + source: io::Error, + }, + RunGitLog { + source: io::Error, + }, + CaptureGitStdout, + GitProcess { + source: io::Error, + }, + GitLogNonZero, + InvalidExcludePattern { + pattern: String, + source: glob::PatternError, + }, + InvalidConfigExcludePattern { + pattern: String, + source: glob::PatternError, + }, +} + +impl fmt::Display for ScannerError { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + ScannerError::DetectorInit { source } => write!(formatter, "{}", source), + ScannerError::Config { source } => write!(formatter, "{}", source), + ScannerError::ReadStream { path, source } => { + write!(formatter, "Read error on {}: {}", path, source) + } + ScannerError::RunGitLog { source } => { + write!(formatter, "Failed to run git log: {}", source) + } + ScannerError::CaptureGitStdout => write!(formatter, "Failed to capture git stdout"), + ScannerError::GitProcess { source } => { + write!(formatter, "git process error: {}", source) + } + ScannerError::GitLogNonZero => write!(formatter, "git log exited with non-zero status"), + ScannerError::InvalidExcludePattern { pattern, source } => { + write!( + formatter, + "Invalid exclude pattern '{}': {}", + pattern, source + ) + } + ScannerError::InvalidConfigExcludePattern { pattern, source } => write!( + formatter, + "Invalid config exclude pattern '{}': {}", + pattern, source + ), + } + } +} + +impl StdError for ScannerError { + fn source(&self) -> Option<&(dyn StdError + 'static)> { + match self { + ScannerError::DetectorInit { source } => Some(source), + ScannerError::Config { source } => Some(source), + ScannerError::ReadStream { source, .. } => Some(source), + ScannerError::RunGitLog { source } => Some(source), + ScannerError::CaptureGitStdout | ScannerError::GitLogNonZero => None, + ScannerError::GitProcess { source } => Some(source), + ScannerError::InvalidExcludePattern { source, .. } + | ScannerError::InvalidConfigExcludePattern { source, .. } => Some(source), + } + } +} diff --git a/templates/pre-push.sh b/templates/pre-push.sh index d7aec3f..ce5b801 100644 --- a/templates/pre-push.sh +++ b/templates/pre-push.sh @@ -134,7 +134,7 @@ main() { fi remote_url=$(resolve_remote_url "$remote_name" "$remote_url_arg") enforce_repository_policy "$remote_url" || exit 1 - "$KEYWATCH_BIN" scan . --exit-mode critical + "$KEYWATCH_BIN" scan . --exit-mode critical --no-config-discovery exit $? } diff --git a/tests/baseline_tests.rs b/tests/baseline_tests.rs index d49c83a..fe2d58f 100644 --- a/tests/baseline_tests.rs +++ b/tests/baseline_tests.rs @@ -1,5 +1,6 @@ -use key_watch::baseline::{Baseline, BaselineEntry}; +use key_watch::baseline::{Baseline, BaselineEntry, BaselineError}; use key_watch::report::{Finding, Severity}; +use std::fs; use tempfile::tempdir; fn make_finding(file: &str, line: usize, ftype: &str, content: &str, plugin: &str) -> Finding { @@ -13,6 +14,33 @@ fn make_finding(file: &str, line: usize, ftype: &str, content: &str, plugin: &st } } +#[test] +fn test_baseline_filters_known_findings() { + let known_finding = make_finding( + "test.txt", + 5, + "AWS Key", + "AKIAIOSFODNN7EXAMPLE", + "AWSAccessKeyDetector", + ); + let baseline = Baseline::from_findings(std::slice::from_ref(&known_finding)); + + let findings = vec![ + known_finding, + make_finding( + "other.txt", + 1, + "API Key", + "sk-abc", + "GenericKeyValueDetector", + ), + ]; + + let filtered = baseline.filter_findings(findings); + assert_eq!(filtered.len(), 1); + assert_eq!(filtered[0].file_path, "other.txt"); +} + #[test] fn test_baseline_filters_moved_finding_in_same_file() { let known_finding = make_finding( @@ -89,14 +117,47 @@ fn test_baseline_save_and_load() -> Result<(), String> { }], }; - baseline.save(&temp_file)?; - let loaded = Baseline::load(&temp_file)?; + baseline + .save(&temp_file) + .map_err(|error| error.to_string())?; + let loaded = Baseline::load(&temp_file).map_err(|error| error.to_string())?; assert_eq!(loaded.entries.len(), 1); assert_eq!(loaded.entries[0].file_path, "a.txt"); assert_eq!(loaded.entries[0].line_number, 1); Ok(()) } +#[test] +fn test_baseline_load_reports_parse_error_for_invalid_json() -> Result<(), String> { + let temp_dir = tempdir().map_err(|err| err.to_string())?; + let temp_file = temp_dir.path().join("baseline.json"); + fs::write(&temp_file, "{not valid json").map_err(|error| error.to_string())?; + + match Baseline::load(&temp_file) { + Err(BaselineError::Parse { path, source }) => { + assert_eq!(path, temp_file); + assert!( + BaselineError::Parse { path, source } + .to_string() + .starts_with("Failed to parse baseline '") + ); + } + other => panic!("expected parse error, got {other:?}"), + } + + Ok(()) +} + +#[test] +fn test_baseline_from_findings() { + let findings = vec![ + make_finding("f1.txt", 1, "A", "x", "D1"), + make_finding("f2.txt", 2, "B", "y", "D2"), + ]; + let baseline = Baseline::from_findings(&findings); + assert_eq!(baseline.entries.len(), 2); +} + #[test] fn test_baseline_from_findings_deduplicates_and_keeps_first_metadata() { let findings = vec![ @@ -109,7 +170,7 @@ fn test_baseline_from_findings_deduplicates_and_keeps_first_metadata() { } #[test] -fn test_baseline_update_merges_new_findings_once() { +fn test_baseline_update_merges_new_findings() { let mut baseline = Baseline::from_findings(&[make_finding("old.txt", 1, "X", "old", "D")]); let new_findings = vec![ make_finding("old.txt", 1, "X", "old", "D"), diff --git a/tests/cli_validation_tests.rs b/tests/cli_validation_tests.rs new file mode 100644 index 0000000..cde1fea --- /dev/null +++ b/tests/cli_validation_tests.rs @@ -0,0 +1,34 @@ +use key_watch::RunCliError; +use key_watch::cli::{CliValidationError, ExitMode, OutputFormat, ScanArgs}; + +#[test] +fn test_stdin_with_path_validation_returns_typed_error() { + let options = ScanArgs { + paths: vec!["secret.txt".to_string()], + stdin: true, + git_history: false, + output: None, + verbose: false, + exclude: None, + exit_mode: ExitMode::Strict, + baseline: None, + update_baseline: false, + config: None, + no_config_discovery: false, + format: OutputFormat::Json, + }; + + let error = options + .validate() + .expect_err("stdin with paths should be rejected"); + + assert_eq!(error, CliValidationError::StdinWithPaths); + assert_eq!(error.to_string(), "Cannot specify both --stdin and paths"); +} + +#[test] +fn test_run_cli_error_wraps_cli_validation_display() { + let error = RunCliError::from(CliValidationError::StdinWithPaths); + + assert_eq!(error.to_string(), "Cannot specify both --stdin and paths"); +} diff --git a/tests/detector_tests.rs b/tests/detector_tests.rs index ad6bfb6..59a6ef1 100644 --- a/tests/detector_tests.rs +++ b/tests/detector_tests.rs @@ -1,4 +1,4 @@ -use key_watch::detector::{Detector, DetectorError}; +use key_watch::detector::{Detector, DetectorError, DetectorInitError}; use key_watch::report::Severity; use std::str::FromStr; @@ -210,3 +210,12 @@ fn test_initialize_detectors_all_names_unique() { ); } } + +#[test] +fn test_detector_init_error_duplicate_name_display() { + let error = DetectorInitError::DuplicateName { + detector: "Dup".to_string(), + }; + + assert_eq!(error.to_string(), "duplicate detector name 'Dup'"); +} diff --git a/tests/hooks_tests.rs b/tests/hooks_tests.rs index 055c481..4730519 100644 --- a/tests/hooks_tests.rs +++ b/tests/hooks_tests.rs @@ -1,5 +1,7 @@ -use key_watch::cli::{CliOptions, Command, ExitMode, HookAction, HookInstallArgs, HookType, Shell}; -use key_watch::hooks::{generate_pre_commit_hook, generate_pre_push_hook}; +use key_watch::cli::{ + CliOptions, CliValidationError, Command, ExitMode, HookAction, HookInstallArgs, HookType, Shell, +}; +use key_watch::{generate_pre_commit_hook, generate_pre_push_hook}; #[cfg(unix)] use std::{ fs, @@ -59,12 +61,12 @@ fn generated_binary_name() -> String { } #[cfg(unix)] -fn run_hook_with_args( +fn run_hook( hook: &str, + hook_args: &[&str], git_script: &str, keywatch_script: &str, cwd: &Path, - hook_args: &[&str], ) -> Output { let bin_dir = cwd.join("bin"); fs::create_dir_all(&bin_dir).expect("create fake bin dir"); @@ -90,8 +92,37 @@ fn run_hook_with_args( } #[cfg(unix)] -fn run_hook(hook: &str, git_script: &str, keywatch_script: &str, cwd: &Path) -> Output { - run_hook_with_args(hook, git_script, keywatch_script, cwd, &[]) +fn run_hook_with_packaged_keywatch(hook: &str, cwd: &Path) -> Output { + let bin_dir = cwd.join("bin"); + fs::create_dir_all(&bin_dir).expect("create fake bin dir"); + write_executable(&bin_dir.join("git"), "#!/bin/bash\nexit 1\n"); + fs::copy( + env!("CARGO_BIN_EXE_key-watch"), + bin_dir.join(generated_binary_name()), + ) + .expect("copy KeyWatch binary"); + fs::copy( + Path::new(env!("CARGO_MANIFEST_DIR")).join("detectors.toml"), + bin_dir.join("detectors.toml"), + ) + .expect("copy packaged detectors"); + + let hook_path = cwd.join("hook.sh"); + fs::write(&hook_path, hook).expect("write hook"); + let path = format!( + "{}:{}", + bin_dir.display(), + std::env::var("PATH").unwrap_or_default() + ); + + std::process::Command::new("bash") + .arg(&hook_path) + .arg("origin") + .current_dir(cwd) + .env("PATH", path) + .env_remove("KEYWATCH_CONFIG_PATH") + .output() + .expect("run hook with packaged KeyWatch") } #[cfg(unix)] @@ -101,7 +132,6 @@ fn keywatch_script_that_records_args(marker: &Path) -> String { marker.display() ) } - #[test] fn test_hook_generation_pre_commit() { let options = hook_install_args(HookType::PreCommit, None, None, Some("*.log,*.tmp")); @@ -189,7 +219,7 @@ fn test_pre_commit_failure_output_does_not_print_matched_secret() { let git_script = "#!/bin/bash\nif [ \"$1\" = \"diff\" ]; then printf 'secret.txt\\0'; exit 0; fi\nexit 1\n"; let keywatch_script = "#!/bin/bash\nprintf 'MATCHED_SECRET_VALUE\\n'\nprintf 'MATCHED_SECRET_VALUE\\n' >&2\nexit 1\n"; - let output = run_hook(&hook, git_script, keywatch_script, &temp_dir); + let output = run_hook(&hook, &[], git_script, keywatch_script, &temp_dir); let combined = format!( "{}{}", String::from_utf8_lossy(&output.stdout), @@ -220,7 +250,7 @@ fn test_pre_commit_skips_staged_symlinks() { let git_script = "#!/bin/bash\nif [ \"$1\" = \"diff\" ]; then printf 'linked.txt\\0'; exit 0; fi\nexit 1\n"; let keywatch_script = format!("#!/bin/bash\nprintf ran > '{}'\nexit 1\n", marker.display()); - let output = run_hook(&hook, git_script, &keywatch_script, &temp_dir); + let output = run_hook(&hook, &[], git_script, &keywatch_script, &temp_dir); assert!( output.status.success(), @@ -249,7 +279,7 @@ fn test_pre_push_allows_normalized_https_and_scp_equivalent() { )); let git_script = "#!/bin/bash\nif [ \"$1\" = \"remote\" ]; then printf 'https://github.com/org/repo.git/\\n'; exit 0; fi\nexit 1\n"; let keywatch_script = "#!/bin/bash\nexit 0\n"; - let output = run_hook(&hook, git_script, keywatch_script, &temp_dir); + let output = run_hook(&hook, &["origin"], git_script, keywatch_script, &temp_dir); assert!( output.status.success(), @@ -273,7 +303,13 @@ fn test_pre_push_allows_https_userinfo_case_and_default_port() { None, )); let git_script = "#!/bin/bash\nif [ \"$1\" = \"remote\" ]; then printf 'https://x-access-token:T@GitHub.COM:443/ORG/REPO.git\\n'; exit 0; fi\nexit 1\n"; - let output = run_hook(&hook, git_script, "#!/bin/bash\nexit 0\n", &temp_dir); + let output = run_hook( + &hook, + &["origin"], + git_script, + "#!/bin/bash\nexit 0\n", + &temp_dir, + ); assert!(output.status.success()); fs::remove_dir_all(&temp_dir).expect("cleanup temp dir"); @@ -293,84 +329,34 @@ fn test_pre_push_blocks_https_userinfo_case_and_default_port() { None, )); let git_script = "#!/bin/bash\nif [ \"$1\" = \"remote\" ]; then printf 'https://user@GitHub.COM:443/ORG/REPO.git\\n'; exit 0; fi\nexit 1\n"; - let output = run_hook(&hook, git_script, "#!/bin/bash\nexit 0\n", &temp_dir); - - assert_eq!(output.status.code(), Some(1)); - fs::remove_dir_all(&temp_dir).expect("cleanup temp dir"); -} - -#[cfg(unix)] -#[test] -fn test_pre_push_rejects_spoofed_substring_remote() { - let temp_dir = unique_temp_dir("pre_push_spoofed_remote"); - let _ = fs::remove_dir_all(&temp_dir); - fs::create_dir_all(&temp_dir).expect("create temp dir"); - - let hook = generate_pre_push_hook(&hook_install_args( - HookType::PrePush, - Some("git@evil.example:org/repo.git"), - None, - None, - )); - let git_script = "#!/bin/bash\nif [ \"$1\" = \"remote\" ]; then printf 'https://evil.example/github.com/org/repo.git\\n'; exit 0; fi\nexit 1\n"; - let keywatch_script = "#!/bin/bash\nexit 0\n"; - let output = run_hook(&hook, git_script, keywatch_script, &temp_dir); - - assert_eq!(output.status.code(), Some(1)); - - fs::remove_dir_all(&temp_dir).expect("cleanup temp dir"); -} - -#[cfg(unix)] -#[test] -fn test_pre_push_blocks_when_actual_pushed_remote_is_unallowed() { - let temp_dir = unique_temp_dir("pre_push_blocks_actual_unallowed"); - let _ = fs::remove_dir_all(&temp_dir); - fs::create_dir_all(&temp_dir).expect("create temp dir"); - - let hook = generate_pre_push_hook(&hook_install_args( - HookType::PrePush, - Some("github.com/org/repo"), - None, - None, - )); - let git_script = "#!/bin/bash\nif [ \"$1\" = \"remote\" ]; then printf 'git@github.com:org/repo.git\\n'; exit 0; fi\nexit 1\n"; - let keywatch_script = "#!/bin/bash\nexit 0\n"; - let output = run_hook_with_args( + let output = run_hook( &hook, + &["origin"], git_script, - keywatch_script, + "#!/bin/bash\nexit 0\n", &temp_dir, - &["mirror", "https://evil.example/org/repo.git"], ); assert_eq!(output.status.code(), Some(1)); - fs::remove_dir_all(&temp_dir).expect("cleanup temp dir"); } #[cfg(unix)] #[test] -fn test_pre_push_blocks_when_actual_pushed_remote_is_blocked() { - let temp_dir = unique_temp_dir("pre_push_blocks_actual_blocked"); +fn test_pre_push_rejects_spoofed_substring_remote() { + let temp_dir = unique_temp_dir("pre_push_spoofed_remote"); let _ = fs::remove_dir_all(&temp_dir); fs::create_dir_all(&temp_dir).expect("create temp dir"); let hook = generate_pre_push_hook(&hook_install_args( HookType::PrePush, + Some("github.com/org/repo"), None, - Some("https://evil.example/org/repo.git"), None, )); - let git_script = "#!/bin/bash\nif [ \"$1\" = \"remote\" ]; then printf 'git@github.com:org/repo.git\\n'; exit 0; fi\nexit 1\n"; + let git_script = "#!/bin/bash\nif [ \"$1\" = \"remote\" ]; then printf 'https://evil.example/github.com/org/repo.git\\n'; exit 0; fi\nexit 1\n"; let keywatch_script = "#!/bin/bash\nexit 0\n"; - let output = run_hook_with_args( - &hook, - git_script, - keywatch_script, - &temp_dir, - &["origin", "https://evil.example/org/repo.git"], - ); + let output = run_hook(&hook, &["origin"], git_script, keywatch_script, &temp_dir); assert_eq!(output.status.code(), Some(1)); @@ -392,7 +378,7 @@ fn test_pre_push_blocks_normalized_https_and_scp_equivalent() { )); let git_script = "#!/bin/bash\nif [ \"$1\" = \"remote\" ]; then printf 'git@github.com:org/repo.git\\n'; exit 0; fi\nexit 1\n"; let keywatch_script = "#!/bin/bash\nexit 0\n"; - let output = run_hook(&hook, git_script, keywatch_script, &temp_dir); + let output = run_hook(&hook, &["origin"], git_script, keywatch_script, &temp_dir); assert_eq!(output.status.code(), Some(1)); @@ -413,18 +399,18 @@ fn test_pre_push_uses_named_remote_push_url_when_argv_url_is_absent() { None, )); let git_script = "#!/bin/bash\nif [ \"$1 $2 $3\" = \"remote get-url --push\" ] && [ \"$4\" = \"mirror\" ]; then printf 'https://push.example/org/repo.git\\n'; exit 0; fi\nif [ \"$1 $2 $3\" = \"remote get-url mirror\" ]; then printf 'https://fetch.example/org/repo.git\\n'; exit 0; fi\nexit 1\n"; - let output = run_hook_with_args( + let output = run_hook( &hook, + &["mirror"], git_script, &keywatch_script_that_records_args(&marker), &temp_dir, - &["mirror"], ); assert!(output.status.success()); assert_eq!( fs::read_to_string(&marker).expect("read scanner args"), - "scan . --exit-mode critical\n" + "scan . --exit-mode critical --no-config-discovery\n" ); fs::remove_dir_all(&temp_dir).expect("cleanup temp dir"); } @@ -442,12 +428,12 @@ fn test_pre_push_falls_back_to_named_remote_fetch_url() { None, )); let git_script = "#!/bin/bash\nif [ \"$1 $2 $3\" = \"remote get-url --push\" ]; then exit 1; fi\nif [ \"$1 $2\" = \"remote get-url\" ] && [ \"$3\" = \"mirror\" ]; then printf 'https://fetch.example/org/repo.git\\n'; exit 0; fi\nexit 1\n"; - let output = run_hook_with_args( + let output = run_hook( &hook, + &["mirror"], git_script, "#!/bin/bash\nexit 0\n", &temp_dir, - &["mirror"], ); assert!(output.status.success()); @@ -467,7 +453,7 @@ fn test_pre_push_ignores_invalid_configured_entries() { None, )); let git_script = "#!/bin/bash\nif [ \"$1\" = \"remote\" ]; then printf 'git@github.com:org/repo.git\\n'; exit 0; fi\nexit 1\n"; - let output = run_hook(&hook, git_script, "#!/bin/bash\nexit 0\n", &temp_dir); + let output = run_hook(&hook, &[], git_script, "#!/bin/bash\nexit 0\n", &temp_dir); assert!(output.status.success()); fs::remove_dir_all(&temp_dir).expect("cleanup temp dir"); @@ -486,7 +472,7 @@ fn test_pre_push_fails_closed_for_unnormalizable_remote_when_filters_exist() { None, )); let git_script = "#!/bin/bash\nif [ \"$1\" = \"remote\" ]; then printf 'not-a-repo\\n'; exit 0; fi\nexit 1\n"; - let output = run_hook(&hook, git_script, "#!/bin/bash\nexit 0\n", &temp_dir); + let output = run_hook(&hook, &[], git_script, "#!/bin/bash\nexit 0\n", &temp_dir); assert_eq!(output.status.code(), Some(1)); assert!( @@ -507,6 +493,7 @@ fn test_pre_push_scans_unnormalizable_remote_when_no_filters_exist() { let git_script = "#!/bin/bash\nif [ \"$1\" = \"remote\" ]; then printf 'not-a-repo\\n'; exit 0; fi\nexit 1\n"; let output = run_hook( &hook, + &[], git_script, &keywatch_script_that_records_args(&marker), &temp_dir, @@ -515,7 +502,70 @@ fn test_pre_push_scans_unnormalizable_remote_when_no_filters_exist() { assert!(output.status.success()); assert_eq!( fs::read_to_string(&marker).expect("read scanner args"), - "scan . --exit-mode critical\n" + "scan . --exit-mode critical --no-config-discovery\n" + ); + fs::remove_dir_all(&temp_dir).expect("cleanup temp dir"); +} + +#[cfg(unix)] +#[test] +fn test_pre_push_ignores_repository_config_that_disables_scanning() { + let temp_dir = unique_temp_dir("pre_push_ignores_repository_config"); + fs::create_dir_all(&temp_dir).expect("create temp dir"); + fs::write(temp_dir.join(".keywatch.toml"), "exclude = [\"**/*\"]\n") + .expect("write repository config"); + fs::write( + temp_dir.join("secret.txt"), + "master_api_key = \"abcdefghijklmnopqrstuvwxyz1234\"\n", + ) + .expect("write critical secret"); + + let hook = generate_pre_push_hook(&hook_install_args(HookType::PrePush, None, None, None)); + let keywatch_script = format!( + "#!/bin/bash\nKEYWATCH_CONFIG_PATH=\"{}\" exec \"{}\" \"$@\"\n", + Path::new(env!("CARGO_MANIFEST_DIR")) + .join("detectors.toml") + .display(), + env!("CARGO_BIN_EXE_key-watch") + ); + let output = run_hook( + &hook, + &["origin"], + "#!/bin/bash\nexit 1\n", + &keywatch_script, + &temp_dir, + ); + + assert_eq!( + output.status.code(), + Some(1), + "repository config must not suppress pre-push scanning: {}", + String::from_utf8_lossy(&output.stderr) + ); + fs::remove_dir_all(&temp_dir).expect("cleanup temp dir"); +} + +#[cfg(unix)] +#[test] +fn test_pre_push_ignores_repository_detector_overrides() { + let temp_dir = unique_temp_dir("pre_push_ignores_repository_detectors"); + fs::create_dir_all(&temp_dir).expect("create temp dir"); + fs::write(temp_dir.join("detectors.toml"), "detectors = []\n") + .expect("write repository detector overrides"); + fs::write( + temp_dir.join("secret.txt"), + "master_api_key = \"abcdefghijklmnopqrstuvwxyz1234\"\n", + ) + .expect("write high-severity secret"); + + let hook = generate_pre_push_hook(&hook_install_args(HookType::PrePush, None, None, None)); + let output = run_hook_with_packaged_keywatch(&hook, &temp_dir); + + assert_eq!( + output.status.code(), + Some(1), + "repository detectors must not suppress pre-push scanning: {}", + String::from_utf8_lossy(&output.stderr) ); fs::remove_dir_all(&temp_dir).expect("cleanup temp dir"); } @@ -536,6 +586,62 @@ fn test_hook_shell_escaping() { ); } +#[cfg(unix)] +#[test] +fn test_pre_push_blocks_unallowed_actual_push_remote_even_when_origin_is_allowed() { + let temp_dir = unique_temp_dir("pre_push_blocks_actual_remote"); + let _ = fs::remove_dir_all(&temp_dir); + fs::create_dir_all(&temp_dir).expect("create temp dir"); + + let hook = generate_pre_push_hook(&hook_install_args( + HookType::PrePush, + Some("github.com/org/repo"), + None, + None, + )); + let git_script = "#!/bin/bash\nif [ \"$1\" = \"remote\" ]; then printf 'https://github.com/org/repo.git\\n'; exit 0; fi\nexit 1\n"; + let keywatch_script = "#!/bin/bash\nexit 0\n"; + let output = run_hook( + &hook, + &["origin", "https://evil.example/acme/blocked.git"], + git_script, + keywatch_script, + &temp_dir, + ); + + assert_eq!(output.status.code(), Some(1)); + + fs::remove_dir_all(&temp_dir).expect("cleanup temp dir"); +} + +#[cfg(unix)] +#[test] +fn test_pre_push_blocks_blocked_actual_push_remote_even_when_origin_is_safe() { + let temp_dir = unique_temp_dir("pre_push_blocks_blocked_actual_remote"); + let _ = fs::remove_dir_all(&temp_dir); + fs::create_dir_all(&temp_dir).expect("create temp dir"); + + let hook = generate_pre_push_hook(&hook_install_args( + HookType::PrePush, + None, + Some("https://evil.example/acme/blocked.git"), + None, + )); + let git_script = "#!/bin/bash\nif [ \"$1\" = \"remote\" ]; then printf 'https://github.com/org/repo.git\n'; exit 0; fi\nexit 1\n"; + let keywatch_script = "#!/bin/bash\nexit 0\n"; + let output = run_hook( + &hook, + &["origin", "https://evil.example/acme/blocked.git"], + git_script, + keywatch_script, + &temp_dir, + ); + + assert_eq!(output.status.code(), Some(1)); + + fs::remove_dir_all(&temp_dir).expect("cleanup temp dir"); +} + #[test] fn test_hook_missing_binary_path() { let options = hook_install_args(HookType::PrePush, None, None, None); @@ -639,6 +745,60 @@ fn test_cli_scan_defaults_to_strict_exit_mode() { } } +#[test] +fn test_cli_scan_can_disable_config_discovery() { + use clap::Parser; + + let options = CliOptions::try_parse_from([ + "key-watch", + "scan", + ".", + "--no-config-discovery", + "--config", + "trusted.toml", + ]) + .expect("trusted explicit config with disabled discovery should parse"); + + match options.command { + Command::Scan(scan_args) => { + assert!(scan_args.no_config_discovery); + assert_eq!(scan_args.config.as_deref(), Some("trusted.toml")); + } + _ => panic!("expected scan command"), + } +} + +#[cfg(unix)] +#[test] +fn test_cli_scan_ignores_repository_config_when_discovery_is_disabled() { + let temp_dir = unique_temp_dir("disabled_config_discovery"); + let _ = fs::remove_dir_all(&temp_dir); + fs::create_dir_all(&temp_dir).expect("create temp dir"); + fs::write(temp_dir.join(".keywatch.toml"), "exclude = [\"**/*\"]\n") + .expect("write repository config"); + fs::write( + temp_dir.join("secret.txt"), + "AWS_ACCESS_KEY_ID=AKIAABCDEFGHIJKLMNOP\n", + ) + .expect("write secret fixture"); + + let output = std::process::Command::new(env!("CARGO_BIN_EXE_key-watch")) + .env( + "KEYWATCH_CONFIG_PATH", + Path::new(env!("CARGO_MANIFEST_DIR")).join("detectors.toml"), + ) + .args([ + "scan", + temp_dir.to_str().expect("temp path should be UTF-8"), + "--no-config-discovery", + ]) + .output() + .expect("run key-watch"); + + assert_eq!(output.status.code(), Some(1)); + fs::remove_dir_all(&temp_dir).expect("cleanup temp dir"); +} + #[test] fn test_cli_pre_commit_rejects_blocked_repos() { use clap::Parser; @@ -656,8 +816,10 @@ fn test_cli_pre_commit_rejects_blocked_repos() { let error = options .validate() .expect_err("pre-commit should reject blocked repos"); - assert!( - error.contains("--allowed-repos and --blocked-repos are only supported for pre-push hooks") + assert_eq!(error, CliValidationError::PreCommitRepositoryFilters); + assert_eq!( + error.to_string(), + "--allowed-repos and --blocked-repos are only supported for pre-push hooks" ); } @@ -698,8 +860,10 @@ fn test_cli_pre_commit_rejects_repo_filters() { let error = options .validate() .expect_err("pre-commit should reject repo filters"); - assert!( - error.contains("--allowed-repos and --blocked-repos are only supported for pre-push hooks") + assert_eq!(error, CliValidationError::PreCommitRepositoryFilters); + assert_eq!( + error.to_string(), + "--allowed-repos and --blocked-repos are only supported for pre-push hooks" ); } @@ -720,7 +884,11 @@ fn test_cli_pre_push_rejects_exclude() { let error = options .validate() .expect_err("pre-push should reject exclude patterns"); - assert!(error.contains("--exclude is only supported for pre-commit hooks")); + assert_eq!(error, CliValidationError::PrePushExclude); + assert_eq!( + error.to_string(), + "--exclude is only supported for pre-commit hooks" + ); } #[test] diff --git a/tests/report_tests.rs b/tests/report_tests.rs index be90b69..605043f 100644 --- a/tests/report_tests.rs +++ b/tests/report_tests.rs @@ -1,4 +1,11 @@ -use key_watch::report::{Finding, ScanMetadata, Severity, create_report, get_severity_counts}; +use key_watch::report::{ + Finding, ScanMetadata, Severity, create_report, create_sarif_report, get_severity_counts, +}; +use serde_json::Value; + +fn parse_json(output: &str) -> Value { + serde_json::from_str(output).expect("valid JSON output") +} #[test] fn test_create_report() { @@ -11,18 +18,13 @@ fn test_create_report() { let report = create_report(findings, metadata, "0.5s".to_string()) .expect("create_report should succeed"); - assert!( - report.contains("\"status\": \"PASS\""), - "Empty findings should be PASS" - ); - assert!( - report.contains("\"files_scanned\": 5"), - "Should include files_scanned" - ); - assert!( - report.contains("\"scan_time\": \"0.5s\""), - "Should include scan_time" - ); + let json = parse_json(&report); + + assert_eq!(json["status"], "PASS"); + assert_eq!(json["files_scanned"], 5); + assert_eq!(json["total_lines"], 100); + assert_eq!(json["excluded_files"], serde_json::json!([])); + assert_eq!(json["scan_time"], "0.5s"); } #[test] @@ -43,11 +45,11 @@ fn test_report_with_findings() { let report = create_report(findings, metadata, "0.1s".to_string()) .expect("create_report should succeed"); - assert!( - report.contains("\"status\": \"FAIL\""), - "Should report FAIL status" - ); - assert!(report.contains("AWS Key"), "Should include finding type"); + let json = parse_json(&report); + + assert_eq!(json["status"], "FAIL"); + assert_eq!(json["findings"][0]["finding_type"], "AWS Key"); + assert_eq!(json["findings"][0]["matched_content"], "AKIATESTKEY"); } #[test] @@ -68,13 +70,156 @@ fn test_create_report_includes_excluded_files_and_plugin_metadata() { let report = create_report(findings, metadata, "1.2s".to_string()) .expect("create_report should succeed"); + let json = parse_json(&report); + + assert_eq!( + json["excluded_files"], + serde_json::json!(["ignored.log", "vendor/secrets.txt"]) + ); + assert_eq!(json["findings"][0]["plugin_name"], "TokenDetector"); + assert_eq!(json["findings"][0]["matched_content"], "tok_test_123"); + assert_eq!(json["total_lines"], 80); +} + +#[test] +fn test_create_sarif_report_uses_camel_case_fields_and_hides_matched_content() { + let secret = "AKIATESTKEY".to_string(); + let findings = vec![Finding { + file_path: "src/secret.txt".to_string(), + line_number: 12, + finding_type: "AWS Key".to_string(), + severity: Severity::Critical, + matched_content: secret.clone(), + plugin_name: "AwsKeyDetector".to_string(), + }]; + let metadata = ScanMetadata { + files_scanned: 1, + total_lines: 12, + excluded_files: vec![], + }; - assert!(report.contains("\"excluded_files\"")); - assert!(report.contains("ignored.log")); - assert!(report.contains("vendor/secrets.txt")); - assert!(report.contains("\"plugin_name\": \"TokenDetector\"")); - assert!(report.contains("\"matched_content\": \"tok_test_123\"")); - assert!(report.contains("\"total_lines\": 80")); + let sarif = create_sarif_report(findings, metadata, "2026-08-01T00:00:00Z".to_string()) + .expect("create_sarif_report should succeed"); + let json = parse_json(&sarif); + + assert_eq!( + json["$schema"], + "https://raw.githubusercontent.com/oasis-tcs/sarif-spec/master/Schemata/sarif-schema-2.1.0.json" + ); + assert_eq!(json["version"], "2.1.0"); + + let driver = &json["runs"][0]["tool"]["driver"]; + assert_eq!(driver["name"], "KeyWatch"); + assert!(driver.get("informationUri").is_some()); + assert!(driver.get("semanticVersion").is_some()); + assert!(driver.get("information_uri").is_none()); + assert!(driver.get("semantic_version").is_none()); + + let result = &json["runs"][0]["results"][0]; + assert_eq!(result["ruleId"], "AWS Key"); + assert_eq!(result["level"], "error"); + assert_eq!(result["message"]["text"], "Potential AWS Key detected"); + assert_eq!( + result["locations"][0]["physicalLocation"]["artifactLocation"]["uri"], + "src/secret.txt" + ); + assert_eq!( + result["locations"][0]["physicalLocation"]["region"]["startLine"], + 12 + ); + assert!(result.get("rule_id").is_none()); + assert!(result.get("physical_location").is_none()); + assert!( + result["locations"][0]["physicalLocation"] + .get("artifact_location") + .is_none() + ); + assert!( + result["locations"][0]["physicalLocation"]["region"] + .get("start_line") + .is_none() + ); + + assert_eq!( + result["properties"]["severity"], + Severity::Critical.as_str() + ); + assert_eq!(result["properties"]["precision"], "very-high"); + assert!(!sarif.contains(&secret)); +} + +#[test] +fn test_create_sarif_report_maps_all_severities_to_expected_levels() { + let findings = vec![ + Finding { + file_path: "critical.txt".to_string(), + line_number: 1, + finding_type: "CriticalRule".to_string(), + severity: Severity::Critical, + matched_content: "critical-secret".to_string(), + plugin_name: "CriticalDetector".to_string(), + }, + Finding { + file_path: "high.txt".to_string(), + line_number: 2, + finding_type: "HighRule".to_string(), + severity: Severity::High, + matched_content: "high-secret".to_string(), + plugin_name: "HighDetector".to_string(), + }, + Finding { + file_path: "medium.txt".to_string(), + line_number: 3, + finding_type: "MediumRule".to_string(), + severity: Severity::Medium, + matched_content: "medium-secret".to_string(), + plugin_name: "MediumDetector".to_string(), + }, + Finding { + file_path: "low.txt".to_string(), + line_number: 4, + finding_type: "LowRule".to_string(), + severity: Severity::Low, + matched_content: "low-secret".to_string(), + plugin_name: "LowDetector".to_string(), + }, + ]; + let metadata = ScanMetadata { + files_scanned: 4, + total_lines: 4, + excluded_files: vec![], + }; + + let sarif = create_sarif_report(findings, metadata, "2026-08-01T00:00:00Z".to_string()) + .expect("create_sarif_report should succeed"); + let json = parse_json(&sarif); + let results = json["runs"][0]["results"] + .as_array() + .expect("results array"); + + let levels: Vec<&str> = results + .iter() + .map(|result| result["level"].as_str().expect("level string")) + .collect(); + assert_eq!(levels, vec!["error", "error", "warning", "note"]); + + let severities: Vec<&str> = results + .iter() + .map(|result| { + result["properties"]["severity"] + .as_str() + .expect("severity string") + }) + .collect(); + assert_eq!( + severities, + vec![ + Severity::Critical.as_str(), + Severity::High.as_str(), + Severity::Medium.as_str(), + Severity::Low.as_str() + ] + ); } #[test] diff --git a/tests/run_cli_error_tests.rs b/tests/run_cli_error_tests.rs new file mode 100644 index 0000000..66a9f14 --- /dev/null +++ b/tests/run_cli_error_tests.rs @@ -0,0 +1,31 @@ +use std::fs; +use std::process::Command; + +#[test] +fn test_scan_stdin_with_path_exits_two_with_validation_error() { + let temp_dir = tempfile::tempdir().expect("Create test dir"); + fs::write( + temp_dir.path().join("secret.txt"), + "AWS_KEY=AKIAIOSFODNN7EXAMPLE", + ) + .expect("Write test file"); + + let output = Command::new(env!("CARGO_BIN_EXE_key-watch")) + .current_dir(temp_dir.path()) + .arg("scan") + .arg("secret.txt") + .arg("--stdin") + .output() + .expect("Run key-watch"); + + assert_eq!( + output.status.code(), + Some(2), + "Should exit 2 for invalid stdin/path usage" + ); + let stderr = String::from_utf8_lossy(&output.stderr); + assert_eq!( + stderr.trim(), + "Error: Cannot specify both --stdin and paths" + ); +} diff --git a/tests/scanner_tests.rs b/tests/scanner_tests.rs index f2ce937..8195ec4 100644 --- a/tests/scanner_tests.rs +++ b/tests/scanner_tests.rs @@ -1,5 +1,5 @@ -use key_watch::cli::{ExitMode, ScanArgs}; -use key_watch::scanner::run_scan; +use key_watch::cli::{ExitMode, OutputFormat, ScanArgs}; +use key_watch::scanner::{ScannerError, run_scan}; use std::env::temp_dir; use std::fs; use std::path::{Path, PathBuf}; @@ -114,9 +114,12 @@ sk-abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWX\n\ exit_mode: ExitMode::Strict, baseline: None, update_baseline: false, + config: None, + no_config_discovery: false, + format: OutputFormat::Json, }; - let (findings, _) = run_scan(&options).expect("run_scan should succeed"); + let (findings, _) = run_scan(&options, None).expect("run_scan should succeed"); assert!(!findings.is_empty(), "Should find secrets"); fs::remove_file(test_file).expect("Cleanup"); @@ -144,9 +147,12 @@ Stripe: sk_test_51ABCDEF12345678901234567890\n\ exit_mode: ExitMode::Strict, baseline: None, update_baseline: false, + config: None, + no_config_discovery: false, + format: OutputFormat::Json, }; - let (findings, _) = run_scan(&options).expect("run_scan should succeed"); + let (findings, _) = run_scan(&options, None).expect("run_scan should succeed"); assert!(!findings.is_empty(), "Should find API tokens"); fs::remove_file(test_file).expect("Cleanup"); @@ -175,9 +181,12 @@ AZURE_STORAGE=DefaultEndpointsProtocol=https;AccountName=examplestore; exit_mode: ExitMode::Strict, baseline: None, update_baseline: false, + config: None, + no_config_discovery: false, + format: OutputFormat::Json, }; - let (findings, _) = run_scan(&options).expect("run_scan should succeed"); + let (findings, _) = run_scan(&options, None).expect("run_scan should succeed"); assert!(!findings.is_empty(), "Should find cloud credentials"); fs::remove_file(test_file).expect("Cleanup"); @@ -207,9 +216,12 @@ b3BlbnNzaC1ldi0xLjAAABgQDQD2FGB3V2t4=\n\ exit_mode: ExitMode::Strict, baseline: None, update_baseline: false, + config: None, + no_config_discovery: false, + format: OutputFormat::Json, }; - let (findings, _) = run_scan(&options).expect("run_scan should succeed"); + let (findings, _) = run_scan(&options, None).expect("run_scan should succeed"); assert!(!findings.is_empty(), "Should find private keys"); fs::remove_file(test_file).expect("Cleanup"); @@ -233,9 +245,12 @@ fn test_multiple_detections_in_line() { exit_mode: ExitMode::Strict, baseline: None, update_baseline: false, + config: None, + no_config_discovery: false, + format: OutputFormat::Json, }; - let (findings, _) = run_scan(&options).expect("run_scan should succeed"); + let (findings, _) = run_scan(&options, None).expect("run_scan should succeed"); assert!( findings.len() >= 2, "Should find multiple secrets on one line" @@ -271,9 +286,12 @@ fn test_directory_scan_with_exclusions() { exit_mode: ExitMode::Strict, baseline: None, update_baseline: false, + config: None, + no_config_discovery: false, + format: OutputFormat::Json, }; - let (findings, metadata) = run_scan(&options).expect("run_scan should succeed"); + let (findings, metadata) = run_scan(&options, None).expect("run_scan should succeed"); assert_eq!( metadata.files_scanned, 2, "Should scan 2 files (.git excluded)" @@ -308,9 +326,12 @@ fn test_exclude_pattern_filtering() { exit_mode: ExitMode::Strict, baseline: None, update_baseline: false, + config: None, + no_config_discovery: false, + format: OutputFormat::Json, }; - let (_findings, metadata) = run_scan(&options).expect("run_scan should succeed"); + let (_findings, metadata) = run_scan(&options, None).expect("run_scan should succeed"); assert!( metadata .excluded_files @@ -323,6 +344,42 @@ fn test_exclude_pattern_filtering() { fs::remove_dir_all(test_dir).expect("Cleanup"); } +#[test] +fn test_invalid_cli_exclude_pattern_returns_typed_error() { + let options = ScanArgs { + paths: vec![".".to_string()], + stdin: false, + git_history: false, + output: None, + verbose: false, + exclude: Some("[".to_string()), + exit_mode: ExitMode::Strict, + baseline: None, + update_baseline: false, + config: None, + no_config_discovery: false, + format: OutputFormat::Json, + }; + + let error = match run_scan(&options, None) { + Ok(_) => panic!("invalid exclude should fail"), + Err(error) => error, + }; + + match &error { + ScannerError::InvalidExcludePattern { pattern, source: _ } => { + assert_eq!(pattern, "["); + } + other_error => panic!("expected invalid exclude pattern error, got {other_error:?}"), + } + assert!( + error + .to_string() + .starts_with("Invalid exclude pattern '[': "), + "legacy display prefix changed: {error}" + ); +} + #[test] fn test_dot_github_directory_is_scanned() { let temp_dir = temp_dir(); @@ -347,9 +404,12 @@ fn test_dot_github_directory_is_scanned() { exit_mode: ExitMode::Strict, baseline: None, update_baseline: false, + config: None, + no_config_discovery: false, + format: OutputFormat::Json, }; - let (findings, metadata) = run_scan(&options).expect("run_scan should succeed"); + let (findings, metadata) = run_scan(&options, None).expect("run_scan should succeed"); assert_eq!(metadata.files_scanned, 1, "Should scan .github files"); assert!(!findings.is_empty(), "Should find secrets inside .github"); @@ -372,9 +432,12 @@ fn test_scan_no_secrets() { exit_mode: ExitMode::Strict, baseline: None, update_baseline: false, + config: None, + no_config_discovery: false, + format: OutputFormat::Json, }; - let (findings, _) = run_scan(&options).expect("run_scan should succeed"); + let (findings, _) = run_scan(&options, None).expect("run_scan should succeed"); assert!(findings.is_empty(), "Should not find secrets in plain text"); fs::remove_file(temp_file).expect("Cleanup"); @@ -398,9 +461,12 @@ fn test_non_utf8_file_handling() { exit_mode: ExitMode::Strict, baseline: None, update_baseline: false, + config: None, + no_config_discovery: false, + format: OutputFormat::Json, }; - let (findings, _) = run_scan(&options).expect("run_scan should succeed"); + let (findings, _) = run_scan(&options, None).expect("run_scan should succeed"); assert!(findings.is_empty(), "Should gracefully handle binary files"); fs::remove_file(test_file).expect("Cleanup"); @@ -428,9 +494,12 @@ fn test_multiple_files_scan() { exit_mode: ExitMode::Strict, baseline: None, update_baseline: false, + config: None, + no_config_discovery: false, + format: OutputFormat::Json, }; - let (findings, metadata) = run_scan(&options).expect("run_scan should succeed"); + let (findings, metadata) = run_scan(&options, None).expect("run_scan should succeed"); assert!( !findings.is_empty(), "Should find secrets in multiple files" @@ -459,9 +528,12 @@ fn test_duplicate_paths_are_scanned_once() { exit_mode: ExitMode::Strict, baseline: None, update_baseline: false, + config: None, + no_config_discovery: false, + format: OutputFormat::Json, }; - let (findings, metadata) = run_scan(&options).expect("run_scan should succeed"); + let (findings, metadata) = run_scan(&options, None).expect("run_scan should succeed"); assert_eq!( metadata.files_scanned, 1, "Duplicate paths should be deduped" @@ -504,9 +576,12 @@ fn test_mixed_file_and_directory_paths_are_scanned_once() { exit_mode: ExitMode::Strict, baseline: None, update_baseline: false, + config: None, + no_config_discovery: false, + format: OutputFormat::Json, }; - let (findings, metadata) = run_scan(&options).expect("run_scan should succeed"); + let (findings, metadata) = run_scan(&options, None).expect("run_scan should succeed"); assert_eq!( metadata.files_scanned, 1, "File should only be scanned once" @@ -541,9 +616,12 @@ fn test_nonexistent_paths_are_ignored_without_counting_as_scanned() { exit_mode: ExitMode::Strict, baseline: None, update_baseline: false, + config: None, + no_config_discovery: false, + format: OutputFormat::Json, }; - let (findings, metadata) = run_scan(&options).expect("run_scan should succeed"); + let (findings, metadata) = run_scan(&options, None).expect("run_scan should succeed"); assert!( findings.is_empty(), "Missing paths should not produce findings" @@ -585,9 +663,12 @@ fn test_explicit_symlink_path_is_skipped() -> Result<(), String> { exit_mode: ExitMode::Strict, baseline: None, update_baseline: false, + config: None, + no_config_discovery: false, + format: OutputFormat::Json, }; - let (findings, metadata) = run_scan(&options)?; + let (findings, metadata) = run_scan(&options, None).expect("run_scan should succeed"); assert!(findings.is_empty(), "Symlink target should not be scanned"); assert_eq!( @@ -627,9 +708,12 @@ fn test_recursive_symlink_path_is_skipped() -> Result<(), String> { exit_mode: ExitMode::Strict, baseline: None, update_baseline: false, + config: None, + no_config_discovery: false, + format: OutputFormat::Json, }; - let (findings, metadata) = run_scan(&options)?; + let (findings, metadata) = run_scan(&options, None).expect("run_scan should succeed"); assert!( findings.is_empty(), @@ -662,9 +746,12 @@ fn test_detect_aadhaar() { exit_mode: ExitMode::Strict, baseline: None, update_baseline: false, + config: None, + no_config_discovery: false, + format: OutputFormat::Json, }; - let (findings, _) = run_scan(&options).expect("run_scan should succeed"); + let (findings, _) = run_scan(&options, None).expect("run_scan should succeed"); let aadhaar_findings: Vec<_> = findings .iter() .filter(|f| f.finding_type == "Aadhaar Card Number") @@ -695,9 +782,12 @@ fn test_detect_voter_id() { exit_mode: ExitMode::Strict, baseline: None, update_baseline: false, + config: None, + no_config_discovery: false, + format: OutputFormat::Json, }; - let (findings, _) = run_scan(&options).expect("run_scan should succeed"); + let (findings, _) = run_scan(&options, None).expect("run_scan should succeed"); let voter_findings: Vec<_> = findings .iter() .filter(|f| f.finding_type == "Voter ID (EPIC)") @@ -725,9 +815,12 @@ fn test_detect_pan_card() { exit_mode: ExitMode::Strict, baseline: None, update_baseline: false, + config: None, + no_config_discovery: false, + format: OutputFormat::Json, }; - let (findings, _) = run_scan(&options).expect("run_scan should succeed"); + let (findings, _) = run_scan(&options, None).expect("run_scan should succeed"); let pan_findings: Vec<_> = findings .iter() .filter(|f| f.finding_type == "PAN Card Number") @@ -755,9 +848,12 @@ fn test_detect_abha() { exit_mode: ExitMode::Strict, baseline: None, update_baseline: false, + config: None, + no_config_discovery: false, + format: OutputFormat::Json, }; - let (findings, _) = run_scan(&options).expect("run_scan should succeed"); + let (findings, _) = run_scan(&options, None).expect("run_scan should succeed"); let abha_findings: Vec<_> = findings .iter() .filter(|f| f.finding_type == "ABHA Health ID") @@ -786,9 +882,12 @@ fn test_multiple_indian_ids() { exit_mode: ExitMode::Strict, baseline: None, update_baseline: false, + config: None, + no_config_discovery: false, + format: OutputFormat::Json, }; - let (findings, _) = run_scan(&options).expect("run_scan should succeed"); + let (findings, _) = run_scan(&options, None).expect("run_scan should succeed"); let finding_types: Vec<_> = findings.iter().map(|f| f.finding_type.clone()).collect(); assert!( @@ -842,9 +941,12 @@ fn test_overlapping_scan_roots_with_exclusions() { exit_mode: ExitMode::Strict, baseline: None, update_baseline: false, + config: None, + no_config_discovery: false, + format: OutputFormat::Json, }; - let (findings, metadata) = run_scan(&options).expect("run_scan should succeed"); + let (findings, metadata) = run_scan(&options, None).expect("run_scan should succeed"); assert!( metadata @@ -883,9 +985,12 @@ AWS Key: AKIAABCDEFGHIJKLMNOP # keywatch:ignore\npassword = 'mySecretPassword'\n exit_mode: ExitMode::Strict, baseline: None, update_baseline: false, + config: None, + no_config_discovery: false, + format: OutputFormat::Json, }; - let (findings, _) = run_scan(&options)?; + let (findings, _) = run_scan(&options, None).expect("run_scan should succeed"); let aws_suppressed = findings .iter() @@ -927,6 +1032,9 @@ fn test_stdin_args_validation() { exit_mode: ExitMode::Strict, baseline: None, update_baseline: false, + config: None, + no_config_discovery: false, + format: OutputFormat::Json, }; assert!(options.validate().is_ok()); @@ -982,6 +1090,9 @@ fn test_git_history_args_validation_allows_zero_or_one_path() { exit_mode: ExitMode::Strict, baseline: None, update_baseline: false, + config: None, + no_config_discovery: false, + format: OutputFormat::Json, }; let one_path = ScanArgs { paths: vec!["/tmp/requested-root".to_string()], @@ -993,6 +1104,9 @@ fn test_git_history_args_validation_allows_zero_or_one_path() { exit_mode: ExitMode::Strict, baseline: None, update_baseline: false, + config: None, + no_config_discovery: false, + format: OutputFormat::Json, }; let two_paths = ScanArgs { paths: vec![ @@ -1007,6 +1121,9 @@ fn test_git_history_args_validation_allows_zero_or_one_path() { exit_mode: ExitMode::Strict, baseline: None, update_baseline: false, + config: None, + no_config_discovery: false, + format: OutputFormat::Json, }; assert!(zero_paths.validate().is_ok());