Skip to content

fix: improve detector quality, config resolution, code quality, and binary detection - #57

Merged
pixincreate merged 4 commits into
masterfrom
fix/config-detector-quality-v2
May 11, 2026
Merged

fix: improve detector quality, config resolution, code quality, and binary detection#57
pixincreate merged 4 commits into
masterfrom
fix/config-detector-quality-v2

Conversation

@pixincreate

Copy link
Copy Markdown
Owner

Summary

Address code review findings from PR #56 with the following fixes:

High Priority

  1. Config path resolution (src/detector.rs)

    • Multi-tier fallback: env var → ~/.config/keywatch/ → exe dir → CWD
    • Fixes config not found when installed via cargo install
  2. Detector quality (detectors.toml)

    • Remove duplicate Google/Firebase/YouTube API key detectors (same pattern)
    • Fix IPAddressDetector to only match valid first octet (1-223)
    • Raise HighEntropyDetector threshold from 32+ to 48+ to avoid SHA1/UUIDs
    • Remove LinkedInSecretDetector (too broad, no specific pattern)

Medium Priority

  1. Scanner deduplication (src/scanner.rs)

    • Use HashMap instead of manual Vec loop for deduplication
  2. Report improvements (src/report.rs, src/lib.rs)

    • Single-pass severity counting instead of triple iteration
    • Add Severity and ScanStatus enums with serde(uppercase)
    • Remove hardcoded "HIGH", "MEDIUM", "LOW", "PASS", "FAIL" strings
  3. Binary file detection (src/scanner.rs)

    • Add is_binary() function to skip binary files
    • Check first 8KB for null bytes to detect binary content

Testing

  • All tests pass
  • cargo fmt / clippy clean

Notes

  • Combined into one PR for simplicity as requested

@pixincreate
pixincreate requested a review from CopilotMay 10, 2026 18:50
@pixincreatepixincreate self-assigned this May 10, 2026
@pixincreatepixincreate added the enhancement New feature or request label May 10, 2026
@pixincreate
pixincreateforce-pushed the fix/config-detector-quality-v2 branch from 3506efb to abd7872CompareMay 10, 2026 18:52
- Priority 1: KEYWATCH_CONFIG_PATH env var (override)
- Priority 2: ~/.config/keywatch/detectors.toml
- Priority 3: executable directory (development)
- Priority 4: CWD (last resort)
Fixes config not found when installed via cargo install.
- Remove duplicate Google/Firebase/YouTube API key detectors (same pattern)
- Fix IPAddressDetector to only match valid first octet (1-223)
- Raise HighEntropyDetector threshold from 32+ to 48+ to avoid SHA1
- Remove LinkedInSecretDetector (too broad, no specific pattern)
- Add CWD fallback in config resolution for test compatibility
Fixes false positives and removes redundant detectors.
- scanner.rs: use HashMap for deduplication instead of manual Vec loop
- report.rs: add Severity and ScanStatus enums with serde(uppercase)
- report.rs: single-pass severity counting instead of triple iteration
- lib.rs: update to use Severity enum in tests and exit code logic
Reduces code complexity and removes hardcoded string values
@pixincreate
pixincreateforce-pushed the fix/config-detector-quality-v2 branch from 2fc9639 to 220f1d2CompareMay 10, 2026 19:02

CopilotAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

CopilotAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 7 out of 8 changed files in this pull request and generated 9 comments.

Comment threadsrc/report.rs
Comment threadsrc/lib.rs
Comment threadsrc/detector.rs Outdated
Comment threadsrc/scanner.rs
Comment threadsrc/scanner.rs Outdated
Comment threadsrc/scanner.rs Outdated
Comment threaddetectors.toml
Comment threadsrc/report.rs
Comment threadsrc/report.rs
@pixincreate
pixincreateforce-pushed the fix/config-detector-quality-v2 branch from 220f1d2 to cba40a0CompareMay 11, 2026 07:13
- scanner.rs: add is_binary() function to check for null bytes
- skip first 8KB of file to detect binary content
- binary files are skipped during scanning to avoid false positives
- update tests to use Severity enum
@pixincreate
pixincreateforce-pushed the fix/config-detector-quality-v2 branch from cba40a0 to 84e2e1eCompareMay 11, 2026 07:16
@pixincreate
pixincreate merged commit e42a0ce into masterMay 11, 2026
5 checks passed
@pixincreate
pixincreate deleted the fix/config-detector-quality-v2 branch May 11, 2026 07:20
pixincreate added a commit that referenced this pull request Jul 29, 2026
- Improve OpenAI API key regex (word boundaries, 48+ chars)
- Add OpenCode API key detector (context-based sk- prefix)
- Add 63+ new prefix-based detectors covering:
- AI services (Anthropic, Cohere, HuggingFace, Mistral, Groq, Replicate, LangSmith)
- Cloud/Infra (Vercel, Netlify, Supabase, Convex)
- Monitoring (Datadog, New Relic, PagerDuty, Sentry, Elastic)
- DevOps (GitLab PATs, Buildkite, TravisCI, Ngrok)
- Messaging (Confluent)
- Productivity (Airtable, HashiCorp Vault, Terraform)
- Remove redundant context-based 32-hex detectors (covered by GenericKeyValueDetector)
- Strip verbose descriptions from detectors.toml
- Fix justfile trailing newline
Refs: #57
pixincreate added a commit that referenced this pull request Aug 2, 2026
* feat: expand detector coverage and fix regex patterns
- Improve OpenAI API key regex (word boundaries, 48+ chars)
- Add OpenCode API key detector (context-based sk- prefix)
- Add 63+ new prefix-based detectors covering:
- AI services (Anthropic, Cohere, HuggingFace, Mistral, Groq, Replicate, LangSmith)
- Cloud/Infra (Vercel, Netlify, Supabase, Convex)
- Monitoring (Datadog, New Relic, PagerDuty, Sentry, Elastic)
- DevOps (GitLab PATs, Buildkite, TravisCI, Ngrok)
- Messaging (Confluent)
- Productivity (Airtable, HashiCorp Vault, Terraform)
- Remove redundant context-based 32-hex detectors (covered by GenericKeyValueDetector)
- Strip verbose descriptions from detectors.toml
- Fix justfile trailing newline
Refs: #57
* fix: add CRITICAL severity support (was silently downgraded to LOW)
- Add Critical variant to Severity enum
- Update from_string() to parse CRITICAL
- Update get_severity_counts() to return 4-tuple (critical, high, medium, low)
- Update calculate_exit_code() to treat Critical like High in critical mode
- Update severity display output to include CRITICAL count
- Update tests for new 4-tuple format
Fixes part of #68
* feat: add baseline suppression for known findings
- Add baseline module (src/baseline.rs) with JSON serialization
- Add --baseline <path> flag to suppress known findings
- Add --update-baseline flag to write baseline from current scan
- Baseline matching uses file_path + line_number + finding_type + plugin_name
- Integration tests for filtering, save/load, and from_findings
Refs #68
* fix: resolve merge conflicts and restore full feature implementation
* chore: put omo in gitignore
* fix: scan_stream chunk overlap, salted SHA-256 baseline, real stdin integration test, graceful git-not-found
* docs(changelog): add PR review fix entries
* refactor: use const for baseline version and domain separator, rename severity param
* fix: stabilize baseline fingerprints
* fix: validate detector severity and names
* fix: harden file and git history scanning
* fix: harden generated git hooks
* style: group Rust imports
* fix: guard Unix-only hook test imports
* refactor: clarify pre-push remote policy shell
* refactor: simplify baseline hashing
* style: use descriptive scanner test errors
pixincreate added a commit that referenced this pull request Aug 2, 2026
* feat: expand detector coverage and fix regex patterns
- Improve OpenAI API key regex (word boundaries, 48+ chars)
- Add OpenCode API key detector (context-based sk- prefix)
- Add 63+ new prefix-based detectors covering:
- AI services (Anthropic, Cohere, HuggingFace, Mistral, Groq, Replicate, LangSmith)
- Cloud/Infra (Vercel, Netlify, Supabase, Convex)
- Monitoring (Datadog, New Relic, PagerDuty, Sentry, Elastic)
- DevOps (GitLab PATs, Buildkite, TravisCI, Ngrok)
- Messaging (Confluent)
- Productivity (Airtable, HashiCorp Vault, Terraform)
- Remove redundant context-based 32-hex detectors (covered by GenericKeyValueDetector)
- Strip verbose descriptions from detectors.toml
- Fix justfile trailing newline
Refs: #57
* fix: add CRITICAL severity support (was silently downgraded to LOW)
- Add Critical variant to Severity enum
- Update from_string() to parse CRITICAL
- Update get_severity_counts() to return 4-tuple (critical, high, medium, low)
- Update calculate_exit_code() to treat Critical like High in critical mode
- Update severity display output to include CRITICAL count
- Update tests for new 4-tuple format
Fixes part of #68
* feat: add baseline suppression for known findings
- Add baseline module (src/baseline.rs) with JSON serialization
- Add --baseline <path> flag to suppress known findings
- Add --update-baseline flag to write baseline from current scan
- Baseline matching uses file_path + line_number + finding_type + plugin_name
- Integration tests for filtering, save/load, and from_findings
Refs #68
* fix: resolve merge conflicts and restore full feature implementation
* chore: put omo in gitignore
* fix: scan_stream chunk overlap, salted SHA-256 baseline, real stdin integration test, graceful git-not-found
* feat: add GitHub Action composite action and Dockerfile
- .github/actions/keywatch-scan/action.yml: composite action that downloads
and runs key-watch on specified paths with configurable exit mode
- Dockerfile: multi-stage build (rust:alpine → alpine:3.21 runtime)
- .dockerignore: exclude build artifacts and non-essential files
- .github/workflows/docker-publish.yml: build & push to GHCR on version tags
* fix: address PR review issues - remove eval, add --locked, non-root user, GH auth, keep Cargo.lock
* docs(changelog): add PR review fix entries
* docs(changelog): add GitHub Action and Docker entries
* chore: bump GitHub Actions to latest versions, minimize Docker image to scratch
- docker-publish.yml: actions/checkout v4→v7, login-action v3→v4.6.0,
metadata-action v5→v6.2.0, build-push-action v6→v7.3.0
- ci.yml, release.yml: actions/checkout v4→v7
- Dockerfile: runtime stage alpine→scratch (static musl binary),
numeric USER 65532:65532, absolute ENTRYPOINT path
* refactor: use const for baseline version and domain separator, rename severity param
* chore: upgrade workflow actions to latest, add git to Docker image
- Dockerfile: install git + ca-certificates on alpine:3.23 runtime so
--git-history scanning and hook installation work in the container
- Dockerfile: copy templates/ (build was missing them, failed at compile)
- Dockerfile: bump build stage to rust:1.97.1-alpine (let-chains in
scanner.rs require rust >= 1.88)
- ci.yml: crate-ci/typos master -> v1.48.0, Swatinem/rust-cache v2.7.0 -> v2.9.1
- release.yml: softprops/action-gh-release v1 -> v3.0.2
* fix: stabilize baseline fingerprints
* fix: validate detector severity and names
* fix: harden file and git history scanning
* fix: harden generated git hooks
* style: group Rust imports
* fix: harden composite scan action
* fix: align Docker and release publishing
* fix: guard Unix-only hook test imports
* refactor: clarify pre-push remote policy shell
* refactor: simplify baseline hashing
* style: use descriptive scanner test errors
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancementNew feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@pixincreate