Skip to content

fix: remove unwraps from rule execution paths#283

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

fix: remove unwraps from rule execution paths#283
peaktwilight merged 1 commit intomainfrom
fix/275-unwrap-audit-followup

Conversation

@peaktwilight
Copy link
Copy Markdown
Collaborator

@peaktwilight peaktwilight commented May 2, 2026

Summary

  • Remove an input-path unwrap() in JS command-injection receiver handling.
  • Remove an input-path unwrap() in Go import alias collection.
  • Replace Cargo PQ seed selection unwrap() with an explicit empty-case guard.

Verification

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

Refs #275

Summary by CodeRabbit

  • Bug Fixes

    • Fixed potential crashes in JavaScript command injection detection rule.
    • Fixed potential crashes in manifest file analysis when evaluating dependency security.
  • Refactor

    • Improved stability of Go import handling to eliminate crash risks.

@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: 349dbd49-3930-4330-b018-80e6c0371bb9

📥 Commits

Reviewing files that changed from the base of the PR and between 85a63ee and 1e0f7bc.

📒 Files selected for processing (3)
  • src/rules/go_taint.rs
  • src/rules/javascript.rs
  • src/rules/manifest.rs

📝 Walkthrough

Walkthrough

The PR replaces three panic-prone unwrap() patterns across different rule implementations with safer alternatives: Go import spec parsing uses direct pattern matching, JavaScript command injection detection uses conditional receiver extraction with early return, and Manifest PQ crypto checking uses fallible seed selection with loop skip.

Changes

Error Hardening in Rule Implementations

Layer / File(s) Summary
Go Import Spec Handling
src/rules/go_taint.rs
Import name field is now matched directly on node.child_by_field_name("name") instead of using name_node.map(...).unwrap(), avoiding panic when name field is absent.
JavaScript Exec Receiver Extraction
src/rules/javascript.rs
Exec call receiver is now conditionally extracted only when rfind('.') returns Some(dot_index), with early return when receiver isn't child_process or known aliases, replacing unconditional unwrap() on dot position.
Manifest Seed Selection
src/rules/manifest.rs
Best PQ seed selection now uses fallible let Some(...) else { continue; } pattern instead of max_by(...).unwrap(), skipping packages when reached_seeds is empty rather than panicking.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Poem

🐰 Three unwraps were causing quite a fright,
So the fox removed them left and right,
Now panics won't happen, the code's more tight,
With pattern matching and early returns bright,
Safe seeds and receivers—a developer's delight! 🌟

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix: remove unwraps from rule execution paths' is directly related to the main changes in the PR, which focus on removing unwrap() calls from three rule execution paths across different files.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ 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-followup

Review rate limit: 8/10 reviews remaining, refill in 11 minutes and 40 seconds.

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

@peaktwilight peaktwilight merged commit c1ef514 into main May 2, 2026
17 checks passed
@peaktwilight peaktwilight deleted the fix/275-unwrap-audit-followup branch May 2, 2026 19:35
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.

1 participant