Fix bash not loading correct profile, upgrade xmlsec1 - #12
Conversation
This reverts commit cc7395b.
| # set -o xtrace | ||
| brew install --overwrite "${packages_to_install[@]}" | ||
| set +o xtrace | ||
| # set +o xtrace |
There was a problem hiding this comment.
Why are these commented out?
There was a problem hiding this comment.
Whoops, this is a mistake. Fixed.
| # # This is a workaround for a problem with the 1.3.7 version of xmlsec1. It forces a downgrade to 1.2.7. | ||
| # # The Atlas Toy IDP uses xmlsec1 to sign the SAML requests. | ||
| # # https://stackoverflow.com/questions/76805174/getting-key-not-found-with-xmlsec1-on-macos | ||
| # local desired_sha="7f35e6ede954326a10949891af2dba47bbe1fc17" tmp_libxmlsec1_path=/tmp/libxmlsec1.rb | ||
| # curl -o "${tmp_libxmlsec1_path}" "https://raw.githubusercontent.com/Homebrew/homebrew-core/${desired_sha}/Formula/libxmlsec1.rb" | ||
| # HOMEBREW_NO_INSTALLED_DEPENDENTS_CHECK=1 brew install --formula "${tmp_libxmlsec1_path}" |
There was a problem hiding this comment.
If we comment something out, then we always add a comment to say why that was commented out, and when the commented-out section can be restored. Because otherwise, it's not clear to anyone why this code has been disabled.
If the answer is "we want to disable this for now, but let's comment it out because we intende to restore it in the future", then please include a comment that explains that, and what the conditions are for restoring the code.
Aside from that, it's not clear that this is a "fix". The problem with libxmlsec1 is still an issue. So, what problem are you attempting to solve with this? Something about openssl@1.1, but why is that a problem?
There was a problem hiding this comment.
I'm not sure how do we proceed with this one.
- If we continue to use
xmlsec1@1.2.7, we will need to includeopenssl@1.1that has already been deprecated fromhomebrew. - If we upgrade
xmlsec1to1.3.7, we can useopenssl@3, but thexmlsec1would not work.
I was thinking the following solution:
- Create another fork to skip the
xmlsec1package for those who don't doAtlasdevelopment. OR - Add a instruction to ask user to comment out specific sections. OR
- Install
openssl@1.1from a 3rd-party forked repo. We can not be 100% sure about the security of this solution. - Build from the source code. Here is the last release of
openssl@1.1: https://github.com/openssl/openssl/releases/tag/OpenSSL_1_1_1w. This solution involves additional engineering efforts.
Looking for some insights here.
There was a problem hiding this comment.
we will need to include openssl@1.1 that has already been deprecated from homebrew.
What is the specific problem? Is homebrew refusing to install xmlsec1 because openssl@1.1 has been marked as deprecated? And if so, can we somehow override that and tell homebrew to install it anyway? (Normally I wouldn't want to do this, but it's not a big security risk in our situation)
There was a problem hiding this comment.
Homebrew no longer installs from local formulas. It looks like we need to do a local tap. I'm researching what this actually means for next steps and potential updates to the script
https://github.com/orgs/Homebrew/discussions/6351#discussioncomment-14257503
$ HOMEBREW_NO_INSTALLED_DEPENDENTS_CHECK=1 brew install --formula "${tmp_libxmlsec1_path}"
==> Auto-updating Homebrew...
Adjust how often this is run with `$HOMEBREW_AUTO_UPDATE_SECS` or disable with
`$HOMEBREW_NO_AUTO_UPDATE=1`. Hide these hints with `$HOMEBREW_NO_ENV_HINTS=1` (see `man brew`).
Error: Homebrew requires formulae to be in a tap, rejecting:
/tmp/libxmlsec1.rb (/private/tmp/libxmlsec1.rb)
To create a tap, run e.g.
brew tap-new <user|org>/<repository>
To create a formula in a tap run e.g.
brew create <url> --tap=<user|org>/<repository>
| if [[ ! -f ~/.bash_profile ]]; then | ||
| # Install skeleton .bashrc if one is not present | ||
| cat >> ~/.bashrc <<"EOF" | ||
| if [ -n "$BASH_VERSION" ]; then | ||
| # include .bashrc if it exists | ||
| if [ -f "$HOME/.bashrc" ]; then | ||
| . "$HOME/.bashrc" | ||
| fi | ||
| fi | ||
| EOF | ||
| fi | ||
|
|
There was a problem hiding this comment.
This looks fine, but should be part of a separate PR.
There was a problem hiding this comment.
ACK, will move it into a separate PR.
3 participants
This PR includes fixes to:
bashnot reading.bashrc. I added a script in.bash_profiletosource.bashrcso thatbash` can read the correct script when starting a new terminal window.openssl@1.1is disabled problem.openssl@1.1is disabled byhomebrewfor security issues.openssl@1.1is required byxmlsec1. Based on the comments, we have some compatibility issue with 1.3.7, @coverprice can you check is the issue still exist or can we update the version to workaround theopenssl@1.1?