feat(website): Add CLI command banner to result page#1127
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the
📝 WalkthroughWalkthroughThis PR refactors the SupportMessage component's styling and DOM structure, then introduces a CLI command generation utility and propagates packOptions through the TryIt component hierarchy to enable users to view and copy generated CLI commands in both success and error states. Changes
Sequence DiagramsequenceDiagram
participant Parent as TryIt
participant Result as TryItResult
participant Content as TryItResultContent
participant Util as generateCliCommand
participant UI as CLI Banner UI
Parent->>Result: pass packOptions prop
Result->>Content: pass packOptions prop
Content->>Util: call generateCliCommand(repositoryUrl, packOptions)
Util-->>Content: return CLI command string
Content->>UI: display command in banner
UI->>Content: user clicks copy button
Content->>Content: copy to clipboard + set commandCopied state
Content->>UI: show "Copied!" feedback
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Summary of ChangesHello @yamadashy, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the user experience on the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
3d15082 to
5fad588
Compare
Deploying repomix with
|
| Latest commit: |
7ea89f7
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://6baee94e.repomix.pages.dev |
| Branch Preview URL: | https://feat-website-cli-command-ban.repomix.pages.dev |
Code ReviewOverall: Clean, well-structured PR that adds a useful CLI command banner to encourage local usage. The shared Issues1. Missing error handling in
2. Potential command injection in DetailsIn Consider sanitizing or escaping shell-special characters, or at minimum wrapping the values in single quotes instead of double quotes to prevent shell expansion. 3. DetailsIn Edit: On closer inspection, the messages array was updated too. No issue here. Suggestions
SummaryGood feature addition. The main concern is the lack of shell-escaping for user-provided patterns in the CLI command string. Everything else looks clean and follows the project's conventions well. Reviewed by Claude |
Code ReviewOverall this is a clean, well-structured PR. The Issues1. Missing
return parts.join(' ');2. Command injection via unsanitized patterns The 3. No unit tests for Per project guidelines, new features should include corresponding unit tests. This utility has clear, testable logic — default omission, flag mapping, pattern quoting — and would benefit from test coverage. Minor NotesDetails
PremortemPotential failure scenarios
|
There was a problem hiding this comment.
Code Review
This pull request introduces a new banner on the result page, displaying the equivalent CLI command for user-selected options, which is a great feature to encourage repomix npm package adoption. However, the generateCliCommand utility is vulnerable to shell command injection because it does not properly escape user-provided inputs. This could allow an attacker to execute malicious commands on a user's local machine via crafted URLs. Additionally, there are areas for improvement regarding error handling for the copy-to-clipboard functionality.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1127 +/- ##
=======================================
Coverage 87.17% 87.17%
=======================================
Files 116 116
Lines 4382 4382
Branches 1019 1019
=======================================
Hits 3820 3820
Misses 562 562 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
website/client/components/Home/TryItResultErrorContent.vue (1)
14-29:⚠️ Potential issue | 🟡 MinorAdd error handling to clipboard operation to maintain consistency with similar functions.
navigator.clipboard.writeTextcan reject due to missing permissions or insecure contexts. This is already handled inTryItResultContent.vue(lines 64-77) and thecopyToClipboardutility (resultViewer.ts lines 15-24); the same pattern should be applied here.🛠️ Suggested fix
const copyCommand = async (event: Event) => { event.preventDefault(); event.stopPropagation(); - await navigator.clipboard.writeText(commandWithRepo.value); - copied.value = true; - setTimeout(() => { - copied.value = false; - }, 2000); + try { + await navigator.clipboard.writeText(commandWithRepo.value); + copied.value = true; + setTimeout(() => { + copied.value = false; + }, 2000); + } catch (err) { + console.error('Failed to copy command:', err); + } };
🤖 Fix all issues with AI agents
In `@website/client/components/utils/cliCommand.ts`:
- Around line 3-52: The generateCliCommand function currently injects
user-controlled strings directly into the shell command; escape repositoryUrl,
includePatterns and ignorePatterns by applying POSIX single-quote escaping
before embedding (e.g., trim the value, replace each single quote ' with '\''
then wrap the whole value in single quotes) and use the escaped values in the
parts.push calls for --remote, --include and --ignore; keep existing logic for
defaults and boolean flags unchanged but ensure you use the escaped variable
names instead of the raw packOptions fields.
🧹 Nitpick comments (1)
website/client/components/Home/SupportMessage.vue (1)
5-25: Consider removing or extracting the commented-out sponsor message.The commented-out sponsor block (lines 6-13) is dead code. If it's intended for future use, consider tracking it in a separate issue or adding a TODO comment explaining when it should be re-enabled. Additionally, with only one message in the array, the random selection logic on line 24 always returns index 0—this could be simplified if multiple messages aren't planned.
Display equivalent CLI command in result page to encourage npm usage. The banner shows the npx command with all selected options, allowing users to easily copy and run the same pack locally. - Add generateCliCommand utility for consistent command generation - Share command generation logic between result and error pages - Style as prominent banner with gradient background
…ions Refactor SupportMessage to use centered banner layout with yellow gradient. Update CLI banner to use simple background with command in white box.
b291e7d to
e274bb2
Compare
Code ReviewOverall this is a well-structured PR that adds a useful CLI command banner. The Security: Command Injection via User InputSeverity: Medium — Both Gemini and CodeRabbit flagged this, and it's a valid concern. The While the generated command is only displayed and copied to clipboard (not executed by the website), a user who copies and pastes it into a terminal could be affected by a crafted URL. For example, a malicious Recommendation: At minimum, wrap user-provided values in single quotes and escape embedded single quotes. Something like: function shellEscape(value: string): string {
return `'${value.replace(/'/g, "'\\''")}'`;
}Then use Minor: Missing error handling in
|
Code ReviewOverall this is a clean, well-structured PR. The Security: Command Injection via Copy-PasteGemini and CodeRabbit flagged this, and it's worth discussing. The However, the severity is lower than typical injection because:
That said, sanitizing these inputs is still recommended as defense-in-depth. A simple approach: validate that Missing Error Handling in TryItResultErrorContent.vueThe Minor ObservationsDetails
Premortem / Edge CasesDetails
Good work — this is a nice UX addition to encourage CLI adoption. |
…nd generator Address PR review comments: - Add shell escaping for user-controlled values (repositoryUrl, includePatterns, ignorePatterns) to prevent command injection when users copy-paste the generated command - Skip --remote flag for uploaded file names by validating with isValidRemoteValue - Add unit tests for generateCliCommand covering all option combinations
Code ReviewThe latest commit addresses the shell escaping and ZIP upload handling concerns from earlier reviews. The code is clean and well-structured. A few remaining observations: Minor Issues1. Details
2. Error page DetailsIn Premortem / Edge Cases
Positive Notes
LGTM with the minor clipboard error handling nit. Reviewed by Claude |
…lint Use local interface definition instead of importing PackOptions from usePackOptions.ts which depends on Vue and fails tsc in CI.
Code Review — ClaudeClean, well-structured PR. The Issues1. Missing try/catch in
Details const copyCommand = async (event: Event) => {
event.preventDefault();
event.stopPropagation();
- await navigator.clipboard.writeText(commandWithRepo.value);
- copied.value = true;
- setTimeout(() => {
- copied.value = false;
- }, 2000);
+ try {
+ await navigator.clipboard.writeText(commandWithRepo.value);
+ copied.value = true;
+ setTimeout(() => {
+ copied.value = false;
+ }, 2000);
+ } catch (err) {
+ console.error('Failed to copy command:', err);
+ }
};2. Shell injection concern is already addressed The Observations
Overall this looks good to merge after the minor clipboard error handling fix. |
Define CliCommandPackOptions interface locally in cliCommand.ts instead of importing PackOptions from usePackOptions.ts which depends on Vue module. This prevents tsc from following the import chain to Vue in CI.
Code ReviewOverall this is a clean, well-structured feature. The Security — Shell Escaping ContextThe Minor Issues1.
|
Add a banner on the result page that displays the equivalent CLI command for running Repomix locally. This encourages website users to try the npm package.
generateCliCommandutility used by both result and error content componentsChecklist
npm run testnpm run lint