-
Notifications
You must be signed in to change notification settings - Fork 52
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
Update Lethe server configuration #4854
Conversation
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.
Hey @aradhanas9 - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟡 Security: 1 issue found
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.
known_hosts: ${{ secrets.KNOWN_HOSTS_OF_BASTION }} | ||
config: ${{ secrets.CONFIG }} | ||
|
||
- name: Install SSH key of target |
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.
question (code_clarification): Clarify the necessity and usage of multiple SSH keys in the workflow.
The introduction of a separate SSH key for the target alongside the bastion key suggests a specific security model. Ensure that the documentation or comments within the workflow clarify why both keys are necessary and how they are used to maintain secure access.
@@ -13,7 +13,7 @@ purge() { | |||
|
|||
echo " - ${version}" | |||
python3 tools/versions.py --delete "${version}" | |||
ssh -o StrictHostKeyChecking=no -T nuxeo@lethe.nuxeo.com "rm -vf ${REMOTE_PATH_PROD}/alpha/*${version}.* ${REMOTE_PATH_PROD}/alpha/*${version}-*" || true | |||
ssh -o StrictHostKeyChecking=no -T lethe.nuxeo.com "rm -vf ${REMOTE_PATH_PROD}/alpha/*${version}.* ${REMOTE_PATH_PROD}/alpha/*${version}-*" || true |
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.
🚨 issue (security): Review the security implications of disabling StrictHostKeyChecking.
Disabling StrictHostKeyChecking can expose the workflow to man-in-the-middle attacks. If this setting is necessary for operational reasons, consider documenting the risks and any mitigating factors in place.
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.
👍
This PR is raised to update lethe server configuration. This is required as Lethe server is migrated to another aws account under private network.