Skip to content

fix: finish non-test unwrap audit#284

Merged
peaktwilight merged 1 commit intomainfrom
fix/275-unwrap-audit-complete
May 2, 2026
Merged

fix: finish non-test unwrap audit#284
peaktwilight merged 1 commit intomainfrom
fix/275-unwrap-audit-complete

Conversation

@peaktwilight
Copy link
Copy Markdown
Collaborator

@peaktwilight peaktwilight commented May 2, 2026

Summary

  • Replace remaining production static-regex unwrap() calls with explicit invariant expect(...) messages.
  • Remove a data-dependent strip_prefix(...).unwrap() in JavaScript taint module export summary extraction.
  • Leave test-only unwraps in place; the production scan/reporting paths are now audited.

Verification

  • cargo fmt --check
  • cargo clippy --all-targets -- -D warnings
  • cargo test

Closes #275

Summary by CodeRabbit

  • Refactor
    • Internal improvements to error handling in regex compilation across multiple security rules.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 2, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 7e5fb03f-d3f4-4975-be0c-5e3baf497275

📥 Commits

Reviewing files that changed from the base of the PR and between c1ef514 and 1764d02.

📒 Files selected for processing (13)
  • src/rules/config.rs
  • src/rules/csharp.rs
  • src/rules/go.rs
  • src/rules/java.rs
  • src/rules/javascript.rs
  • src/rules/javascript_taint.rs
  • src/rules/kotlin.rs
  • src/rules/php.rs
  • src/rules/python.rs
  • src/rules/ruby.rs
  • src/rules/rust_lang.rs
  • src/rules/swift.rs
  • src/secrets.rs

📝 Walkthrough

Walkthrough

This PR systematically replaces .unwrap() calls with .expect() for Regex::new() invocations across the codebase, providing explicit panic messages when regex compilation fails. A single logic refinement in javascript_taint.rs uses strip_prefix with if let instead of starts_with() plus unwrap().

Changes

Regex Compilation Robustness

Layer / File(s) Summary
Static Regex Message Improvements
src/rules/config.rs, src/rules/csharp.rs, src/rules/go.rs, src/rules/java.rs, src/rules/kotlin.rs, src/rules/php.rs, src/rules/python.rs, src/rules/ruby.rs, src/rules/rust_lang.rs, src/rules/swift.rs
All Regex::new(...) compilations in rule implementations switch from .unwrap() to .expect("static [language] [rule] regex should compile") for clearer panic messages on regex construction failure. Matching logic and rule detection remain unchanged.
Secret Detection Pattern Improvements
src/secrets.rs
All secret-detection regex patterns switch from .unwrap() to .expect("static [secret type] regex should compile"), maintaining pattern logic and detection behavior while improving failure diagnostics.
String Extraction Refinement
src/rules/javascript_taint.rs
scan_module_exports refactors module name extraction from starts_with(...).unwrap() to strip_prefix(...) with if let binding, making the string prefix handling more idiomatic while preserving export name assignment and taint traversal logic.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

Poem

🐰 A hop through unwrap's lonely lane,
Expect now whispers why the pain,
Each regex stands with words so clear—
No silent panics need we fear!
Strip the prefix, keep the flow,
Robustness seeds begin to grow!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix: finish non-test unwrap audit' clearly summarizes the main change: completing the replacement of unwrap() calls with expect() in production code.
Linked Issues check ✅ Passed The PR successfully addresses issue #275 objectives by replacing unwrap() calls with expect() containing explicit messages throughout multiple rule files and secrets.rs, demonstrating adherence to the audit completion goal.
Out of Scope Changes check ✅ Passed All changes are scoped to replacing unwrap() with expect() in production code, which directly aligns with the linked issue #275 requirements.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/275-unwrap-audit-complete

Review rate limit: 7/10 reviews remaining, refill in 14 minutes and 17 seconds.

Comment @coderabbitai help to get the list of available commands and usage tips.

@peaktwilight peaktwilight merged commit 0cbaa69 into main May 2, 2026
17 checks passed
@peaktwilight peaktwilight deleted the fix/275-unwrap-audit-complete branch May 2, 2026 19:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

robustness: audit .unwrap() calls in non-test code

1 participant