Skip to content

Conversation

@son-oz
Copy link
Contributor

@son-oz son-oz commented Oct 1, 2025

Summary

  • Ignore detection of unmaintained crates
  • Fix some vulnerabilities in the plugins folder

Testing Process

Checklist

  • Add a reference to related issues in the PR description.
  • Add unit tests if applicable.

Summary by CodeRabbit

  • Security

    • Enforced patched versions of transitive dependencies (e.g., axios and tmp) to reduce exposure to known issues.
    • Updated vulnerability scanner configuration to ignore select, acceptable advisories while keeping existing ignores intact.
  • Chores

    • Declared the workspace package manager to ensure consistent installs across environments.
    • Introduced workspace configuration to control dependency resolution behavior.

@son-oz son-oz requested review from a team as code owners October 1, 2025 22:29
@coderabbitai
Copy link

coderabbitai bot commented Oct 1, 2025

Walkthrough

Adds three RUSTSEC vulnerabilities to the ignore list in osv-scanner.toml. Introduces a packageManager field in plugins/package.json pointing to a specific pnpm version. Creates plugins/pnpm-workspace.yaml to enforce dependency version overrides for axios and tmp within the pnpm workspace.

Changes

Cohort / File(s) Change summary
Security scanning config
osv-scanner.toml
Added IgnoredVulns entries for RUSTSEC-2021-0141, RUSTSEC-2024-0375, and RUSTSEC-2024-0388; existing RUSTSEC-2024-0436 remains unchanged.
Package manager metadata
plugins/package.json
Added top-level "packageManager" set to [email protected]+sha512...; adjusted JSON trailing comma/brace accordingly.
PNPM workspace overrides
plugins/pnpm-workspace.yaml
New workspace config adding overrides that constrain axios to >=1.12.0 when older ranges are requested and tmp to include >=0.2.4 when ranges are <=0.2.3.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Dev as Developer
  participant PNPM as pnpm CLI
  participant Resolver as pnpm Resolver
  participant Reg as Registry
  participant WS as pnpm-workspace.yaml
  participant Pkg as package.json
  Dev->>PNPM: pnpm install
  PNPM->>Pkg: Read dependencies
  PNPM->>WS: Load overrides
  note over WS,Resolver: Apply axios >=1.12.0 and tmp >=0.2.4 constraints
  PNPM->>Resolver: Resolve versions with overrides
  Resolver->>Reg: Fetch matching tarballs
  Reg-->>Resolver: Packages
  Resolver-->>PNPM: Locked dependency graph
  PNPM-->>Dev: Installed workspace with overrides enforced
Loading
sequenceDiagram
  autonumber
  actor CI as CI
  participant Scanner as osv-scanner
  participant Conf as osv-scanner.toml
  CI->>Scanner: Run vulnerability scan
  Scanner->>Conf: Load IgnoredVulns
  note over Scanner,Conf: Ignore RUSTSEC-2021-0141, -2024-0375, -2024-0388, -2024-0436
  Scanner-->>CI: Report with specified advisories suppressed
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I thump the ground—new configs bloom,
pnpm tracks the proper plume.
Overrides hop, axios and tmp align,
the scanner blinks, four rust warnings benign.
In tidy burrows, deps stay neat—
a rabbit’s repo, dependably sweet. 🐇✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Description Check ⚠️ Warning While the description follows the repository’s template by including “# Summary,” “## Testing Process,” and “## Checklist” headings, the Testing Process section is empty and checklist items for linking related issues and adding unit tests remain unaddressed, leaving critical context and validation steps unspecified. Please provide details under “## Testing Process” describing how the changes were verified, add references to any related issue tickets, and implement or link to unit tests where applicable to complete the checklist before merging.
Title Check ❓ Inconclusive The title “chore(deps): Fix vulnerabilites” does indicate that the PR addresses dependency-related vulnerability fixes, but the typo in “vulnerabilites” and the lack of specifics about which vulnerabilities or components were updated make it too generic and less clear for maintainers scanning the history. Please correct the spelling of “vulnerabilites” to “vulnerabilities” and consider including the key change (for example, “Ignore unmaintained crates and add pnpm vulnerability overrides”) to more precisely reflect the PR’s purpose.
✅ Passed checks (1 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch ps-25-add-ignored-vulns

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 971ab9b and f3fef46.

⛔ Files ignored due to path filters (1)
  • plugins/pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (3)
  • osv-scanner.toml (1 hunks)
  • plugins/package.json (1 hunks)
  • plugins/pnpm-workspace.yaml (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
  • GitHub Check: boostsecurity - boostsecurityio/semgrep-pro
  • GitHub Check: Redirect rules - openzeppelin-relayer
  • GitHub Check: Header rules - openzeppelin-relayer
  • GitHub Check: Pages changed - openzeppelin-relayer
  • GitHub Check: Analyze (rust)
🔇 Additional comments (1)
plugins/pnpm-workspace.yaml (1)

2-4: Overrides not applied due to invalid pnpm-workspace structure.

pnpm-workspace.yaml only honors overrides nested under the pnpm key. With overrides at the top level, pnpm ignores these constraints, so axios/tmp stay vulnerable. Move them under pnpm.overrides so they actually take effect.

Apply this diff to fix the structure:

 ---
-overrides:
-  axios@<1.12.0: '>=1.12.0'
-  tmp@<=0.2.3: '>=0.2.4'
+pnpm:
+  overrides:
+    axios@<1.12.0: '>=1.12.0'
+    tmp@<=0.2.3: '>=0.2.4'

Likely an incorrect or invalid review comment.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@@ -0,0 +1,4 @@
---
overrides:
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not a fan of using overrides because they're often forgotten. What we should do is add comments explaining why we're adding these overrides, like which dependencies require them. That way, once those dependencies are updated, we can remove the overrides.

@@ -0,0 +1,4 @@
---
Copy link
Contributor

Choose a reason for hiding this comment

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

What if we also add minimumReleaseAge to not be that vulnerable to supply chain attacks. 1440 seems like a good value, which is 24h.
https://pnpm.io/settings#minimumreleaseage

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah I've looked into it, I'm gonna work on a plan to harden supply chain security across our repos, including settings like this.

Copy link
Contributor

@tirumerla tirumerla left a comment

Choose a reason for hiding this comment

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

lgtm, agree with suggestions that @LuisUrrutia raised.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants