Skip to content
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

Fix interactive install script #8669

Merged
merged 5 commits into from
Nov 22, 2024
Merged

Fix interactive install script #8669

merged 5 commits into from
Nov 22, 2024

Conversation

FelixMalfait
Copy link
Member

Second attempt to fix the interactive install script, by downloading the file first and then executing it

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

This PR modifies the installation process to improve security by downloading and verifying the install script before execution, rather than piping directly to bash.

  • Modified /packages/twenty-docker/scripts/1-click.sh to download script to twenty_install.sh instead of direct pipe to bash
  • Added version comparison logic to handle script location change in v0.32.4
  • Added chmod +x command to ensure script is executable before running
  • Added cleanup step to remove downloaded script after execution
  • Maintained backward compatibility by supporting both old and new script locations

1 file(s) reviewed, 3 comment(s)
Edit PR Review Bot Settings | Greptile

Comment on lines 9 to 11
curl -L "https://raw.githubusercontent.com/twentyhq/twenty/$pull_branch/packages/twenty-docker/scripts/install.sh" -o twenty_install.sh
else
curl -sL "https://raw.githubusercontent.com/twentyhq/twenty/$pull_branch/install.sh" | bash -- "$VERSION" "$BRANCH"
curl -sL "https://raw.githubusercontent.com/twentyhq/twenty/$pull_branch/install.sh" -o twenty_install.sh
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: Missing error handling for failed curl downloads. Script continues even if download fails.

Comment on lines 14 to 15
chmod +x twenty_install.sh
./twenty_install.sh "$VERSION" "$BRANCH"
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: Missing check if twenty_install.sh exists before chmod and execution

chmod +x twenty_install.sh
./twenty_install.sh "$VERSION" "$BRANCH"

rm twenty_install.sh
Copy link
Contributor

Choose a reason for hiding this comment

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

style: rm should use -f flag to prevent errors if file doesn't exist

@FelixMalfait FelixMalfait merged commit 0cd8e61 into main Nov 22, 2024
12 checks passed
@FelixMalfait FelixMalfait deleted the fix-interactive-script branch November 22, 2024 09:29
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