Skip to content

Fix bash not loading correct profile, upgrade xmlsec1 #12

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.

Sign up for GitHub

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

Open
tangyisheng2 wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Fix bash not loading correct profile, upgrade xmlsec1 #12

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
305dc26
Add logic to load .bashrc in .bash_profile
tangyisheng2 May 29, 2025
cc7395b
Remove exact version requirements of xmlsec1:
tangyisheng2 May 29, 2025
2308561
Revert "Remove exact version requirements of xmlsec1:"
tangyisheng2 May 29, 2025
e23f7fe
Revert comments
tangyisheng2 May 29, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 18 additions & 6 deletions install_packages.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,12 +152,12 @@ function macos_install() {
brew services start redis
brew services start "postgresql@15"

# 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}"
# # 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}"
Comment on lines +155 to +160

@coverprice coverprice May 29, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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?

@tangyisheng2 tangyisheng2 May 30, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure how do we proceed with this one.

  • If we continue to use xmlsec1@1.2.7 , we will need to include openssl@1.1 that has already been deprecated from homebrew.
  • If we upgrade xmlsec1 to 1.3.7, we can use openssl@3, but the xmlsec1 would not work.

I was thinking the following solution:

  1. Create another fork to skip the xmlsec1 package for those who don't do Atlas development. OR
  2. Add a instruction to ask user to comment out specific sections. OR
  3. Install openssl@1.1 from a 3rd-party forked repo. We can not be 100% sure about the security of this solution.
  4. 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.

@coverprice coverprice May 30, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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)

@paderobins paderobins Nov 18, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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>

}


Expand All @@ -184,6 +184,18 @@ unset rc
EOF
fi

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

Comment on lines +187 to +198

@coverprice coverprice May 29, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This looks fine, but should be part of a separate PR.

@tangyisheng2 tangyisheng2 May 30, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

ACK, will move it into a separate PR.

# Stop the annoying zsh default shell warning
if [[ ! -f ~/.bash_profile ]] || ! grep -E BASH_SILENCE_DEPRECATION_WARNING ~/.bash_profile >/dev/null 2>&1 ; then
echo "export BASH_SILENCE_DEPRECATION_WARNING=1" >> ~/.bash_profile
Expand Down