-
-
Notifications
You must be signed in to change notification settings - Fork 108
Update install command #162
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| 20 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,7 +7,13 @@ import { Aside } from "@astrojs/starlight/components"; | |
|
|
||
| ### Install the client | ||
|
|
||
| Install the client using homebrew: | ||
| Using the install script: | ||
|
|
||
| ```shell | ||
| curl -sSf https://install.portr.dev | sh | ||
| ``` | ||
|
|
||
| Or install the client using homebrew: | ||
|
Comment on lines
+10
to
+16
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Verification agent 🧩 Analysis chainAddress the grammar issue and consider security documentation. The documentation structure aligns well with the UI changes, but there's a grammar issue flagged by static analysis. Also consider adding security guidance for the curl-based install method. Apply this diff to fix the grammar and add security context: -Using the install script:
+Using the installation script:
+:::caution[Security Notice]
+The install script downloads and executes code directly. For enhanced security, consider downloading and inspecting the script first:
+
Let me verify the grammar concern from static analysis:
---
🏁 Script executed:
```shell
curl -sSf https://install.portr.dev | shLength of output: 959 🏁 Script executed: +curl -sSf https://install.portr.dev -o install.sh && chmod +x install.sh && ./install.sh
+```
+:::
Or install the client using homebrew:Length of output: 265 🌐 Web query: 💡 Result: Both "install script" and "installation script" can be used, but they have slightly different connotations and usage preferences.
In most professional and technical contexts, "installation script" is preferred for clarity and formality. Use “installation script” and add security guidance Tested the direct install script; it runs as expected. To improve clarity and help users stay safe, update the phrasing and include a caution block: Suggested patch: - Using the install script:
+ Using the installation script:
```shell
curl -sSf https://install.portr.dev | sh
Or install the client using Homebrew: In docs/src/content/docs/client/installation.mdx around lines 10 to 16, replace |
||
|
|
||
| ```shell | ||
| brew install amalshaji/taps/portr | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Consider security implications of the curl-based install script.
The new install script pattern
curl -sSf https://install.portr.dev | shis convenient but poses security risks since it downloads and executes code directly. Consider adding a security warning or offering users the option to download and inspect the script first.Consider adding a security notice in the UI or providing an alternative download-first approach:
Let me verify if the install script URL is accessible:
🏁 Script executed:
Length of output: 1606
Add security notice and download-first option for the curl installer
The install script at
https://install.portr.devis accessible and returns the expected bash installer. However, piping remote code directly intoshcarries inherent risks. Please update the UI and constants to:• Offer a download-and-inspect approach as the recommended path
• Retain the direct pipeline option with a clear warning
Locations to update:
Suggested diff:
Additionally, consider adding a brief security notice in the UI adjacent to the install command, e.g.:
“Running remote scripts directly is less secure. We recommend downloading and reviewing the script before execution.”
📝 Committable suggestion
🤖 Prompt for AI Agents