fix: Modal hook order + plugin install slug check#200
fix: Modal hook order + plugin install slug check#200github-actions[bot] merged 1 commit intodevelopfrom
Conversation
- Modal: move useId() before `if (!open) return null` to prevent React hook order violation on open/close toggle - installFromUrl: remove strict manifest.id === pluginSlug check since marketplace slug and manifest id are not guaranteed to match Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 31 minutes and 50 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b73e96eb8b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| // Install plugin from a remote URL (marketplace download) | ||
| ipcMain.handle('plugins:installFromUrl', async (_event, url: string, pluginSlug: string) => { | ||
| ipcMain.handle('plugins:installFromUrl', async (_event, url: string, _pluginSlug: string) => { |
There was a problem hiding this comment.
Validate downloaded manifest ID against requested plugin
By ignoring pluginSlug in plugins:installFromUrl, the install path is now bound only to manifest.id from the downloaded archive. If the marketplace entry is misconfigured (or serves the wrong bundle), installing plugin A can silently delete/replace plugin B because destDir is computed from manifest.id and existing directories are removed before rename. The previous check prevented this cross-plugin overwrite scenario, so we still need an integrity check tying the requested plugin identity to the downloaded manifest.
Useful? React with 👍 / 👎.
## Summary Addresses P1 review finding from PR #200 and strengthens git flow rules. ### Plugin Install - Add cross-plugin overwrite protection: when marketplace serves wrong bundle (manifest.id ≠ requested slug), block only if it would overwrite an existing plugin directory — prevents silent overwrites without blocking valid installs where slug and manifest ID legitimately differ ### CLAUDE.md - Add explicit step-by-step workflow for Claude Code - Add explicit NEVER list (no direct commits to develop/main) ## Test plan - [x] `pnpm typecheck` — 17/17 pass 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Enhanced plugin installation safety to prevent accidental overwrites of existing plugins during installation from URL. * **Documentation** * Updated development workflow documentation and branch protection requirements. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
#208) ## Release Summary Major release with all review findings resolved and conflicts synced. ### Highlights - Design system primitives (Button, Toast, Modal) - Advanced search filters + functional plugin marketplace - High-quality table rendering (editor = preview parity) - Document export with YAML frontmatter + per-note export - Welcome screen, save indicator, update banner, sync progress - Security hardening (plugin install, encryption, YAML/HTML escaping) - A11y improvements (aria-pressed, focus management, live regions) - ESLint typed linting (0 errors), TypeScript strict across all 16 packages - All review findings from PRs #194, #198, #200, #202 addressed ## Test plan - [x] `pnpm typecheck` — 17/17 pass - [x] `pnpm test` — 16/16 pass - [x] `pnpm lint` — 0 errors - [x] Conflicts with main resolved in merge commit 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Summary
Fixes two review findings from PR #198:
useId()above early return in Modal to prevent React hook order violationplugins:installFromUrl— marketplace slug and manifest id are independent identifiersTest plan
pnpm typecheck— 17/17 passpnpm test— 16/16 pass🤖 Generated with Claude Code