Skip to content

feat: fix path injection on macos 15 #841

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

Merged
merged 4 commits into from
Nov 1, 2025
Merged

feat: fix path injection on macos 15 #841

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8406502
feat: fix path injection on macos 15
d12frosted Oct 17, 2025
755a86a
feat: allow to disable path injection before running Emacs
d12frosted Oct 17, 2025
80321b2
fix portability and escaping issues
d12frosted Oct 18, 2025
c699f03
escape single quotes in PATH injection
d12frosted Oct 31, 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
3 changes: 2 additions & 1 deletion Formula/emacs-plus@28.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,8 @@ def caveats
To link the application to default Homebrew App location:
osascript -e 'tell application "Finder" to make alias file to posix file "#{prefix}/Emacs.app" at posix file "/Applications" with properties {name:"Emacs.app"}'

Your PATH value was injected into Emacs.app/Contents/Info.plist
Your PATH value was injected into Emacs.app via a wrapper script.
This solves the issue with macOS Sequoia ignoring LSEnvironment in Info.plist.

Report any issues to https://github.com/d12frosted/homebrew-emacs-plus
EOS
Expand Down
3 changes: 2 additions & 1 deletion Formula/emacs-plus@29.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,8 @@ def caveats
To link the application to default Homebrew App location:
osascript -e 'tell application "Finder" to make alias file to posix file "#{prefix}/Emacs.app" at posix file "/Applications" with properties {name:"Emacs.app"}'

Your PATH value was injected into Emacs.app/Contents/Info.plist
Your PATH value was injected into Emacs.app via a wrapper script.
This solves the issue with macOS Sequoia ignoring LSEnvironment in Info.plist.

Report any issues to https://github.com/d12frosted/homebrew-emacs-plus
EOS
Expand Down
3 changes: 2 additions & 1 deletion Formula/emacs-plus@30.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,8 @@ def caveats
To link the application to default Homebrew App location:
osascript -e 'tell application "Finder" to make alias file to posix file "#{prefix}/Emacs.app" at posix file "/Applications" with properties {name:"Emacs.app"}'

Your PATH value was injected into Emacs.app/Contents/Info.plist
Your PATH value was injected into Emacs.app via a wrapper script.
This solves the issue with macOS Sequoia ignoring LSEnvironment in Info.plist.

Report any issues to https://github.com/d12frosted/homebrew-emacs-plus
EOS
Expand Down
3 changes: 2 additions & 1 deletion Formula/emacs-plus@31.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,8 @@ def caveats
To link the application to default Homebrew App location:
osascript -e 'tell application "Finder" to make alias file to posix file "#{prefix}/Emacs.app" at posix file "/Applications" with properties {name:"Emacs.app"}'

Your PATH value was injected into Emacs.app/Contents/Info.plist
Your PATH value was injected into Emacs.app via a wrapper script.
This solves the issue with macOS Sequoia ignoring LSEnvironment in Info.plist.

Report any issues to https://github.com/d12frosted/homebrew-emacs-plus
EOS
Expand Down
25 changes: 18 additions & 7 deletions Library/EmacsBase.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,20 +31,31 @@ def self.inject_icon_options
end

def inject_path
ohai "Injecting PATH value to Emacs.app/Contents/Info.plist"
ohai "Injecting PATH via wrapper script in Emacs.app/Contents/MacOS/Emacs"
app = "#{prefix}/Emacs.app"
plist = "#{app}/Contents/Info.plist"
emacs_binary = "#{app}/Contents/MacOS/Emacs"
emacs_real = "#{app}/Contents/MacOS/Emacs-real"
Copy link

@glyph glyph Oct 17, 2025

Choose a reason for hiding this comment

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

Computing the absolute path here makes the app bundle non-relocatable. No need to introduce more problems here.

Copy link

@glyph glyph Oct 17, 2025

Choose a reason for hiding this comment

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

to clarify, what I mean is that the script can itself compute a relative path internally, with something like "$(dirname $0)/Emacs-real", which avoids the need for having emacs_real defined at all in Ruby.

path = PATH.new(ORIGINAL_PATHS)

puts "Patching plist at #{plist} with following PATH value:"
puts "Creating wrapper script with following PATH value:"
path.each_entry { |x|
puts x
}

system "/usr/libexec/PlistBuddy -c 'Add :LSEnvironment dict' '#{plist}'"
system "/usr/libexec/PlistBuddy -c 'Add :LSEnvironment:PATH string' '#{plist}'"
system "/usr/libexec/PlistBuddy -c 'Set :LSEnvironment:PATH #{path}' '#{plist}'"
system "/usr/libexec/PlistBuddy -c 'Print :LSEnvironment' '#{plist}'"
# Rename original binary
File.rename(emacs_binary, emacs_real) unless File.exist?(emacs_real)

# Create wrapper script
File.open(emacs_binary, "w") do |f|
f.write <<~EOS
#!/bin/bash
Copy link

@skarekrow skarekrow Oct 17, 2025

Choose a reason for hiding this comment

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

Suggested change
#!/bin/bash
#!/usr/bin/env bash

I think this would be a better idea to allow newer bash versions as well.

Copy link
Owner Author

@d12frosted d12frosted Oct 17, 2025

Choose a reason for hiding this comment

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

not sure there is difference; if Emacs is run via launchctl (finder, spotlight, etc), it will be built-in bash anyways

this actually makes me wonder if we need to provide an env variable to skip setting PATH here - for cases when you want run Emacs from terminal, but... oh maybe I also need to make emacs bin to run original Emacs executable instead of this modified version.

Copy link

@skarekrow skarekrow Oct 17, 2025

Choose a reason for hiding this comment

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

oh fair point! I guess I just knee jerk look for that shebang lol. I agree having an opt out knob in the form of something like EMACS_PLUS_NO_PATH_INJECTION even if overly verbose is a good idea!

Copy link
Owner Author

@d12frosted d12frosted Oct 17, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

cool, I am on a party right now, so not super focused
hence - all feedback and comments are appreciated even more than usually ❤️

p.s. pushed ability to dynamically disable path injection

Copy link

@glyph glyph Oct 17, 2025

Choose a reason for hiding this comment

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

It would probably be more correct here to use #!/bin/sh . macOS does unfortunately depend on bash pretty closely, but /bin/sh is a slightly leaner and more standards-compliant build of bash which will make startup a little faster. Apple has been (extremely slowly, since it's such a core system component) moving away from bash by steering users to zsh as the default login shell, so we can expect it'll go away eventually.

Copy link

@skarekrow skarekrow Oct 17, 2025

Choose a reason for hiding this comment

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

Nice! I really appreciate the work keeping this project alive for emacs on macOS. Thanks for all the time you spend on it! PR LGTM!

export PATH="#{path}"
exec "#{emacs_real}" "$@"
Copy link

@glyph glyph Oct 17, 2025

Choose a reason for hiding this comment

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

If the users's homebrew path has a shell metacharacter in it you'll trigger the same kinds of bugs as above.

Copy link
Owner Author

@d12frosted d12frosted Oct 18, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Ah, disregard, it was about original exec "#{emacs_real}" "$@" and not the proposed code. I will update PR after I test that it works.

Not sure I understand you. Are you talking about a situation when someone installed Homebrew in a path like:

  • /path with spaces/homebrew
  • /foo$bar/homebrew
  • /some/pathwithbackticks/homebrew

If that's what you are talking about, then let's trace through what happens:

  1. $0 = path to the wrapper script (e.g., /opt/homebrew/.../Emacs)
  2. dirname "$0" = directory containing the script
  3. $(dirname "$0")/Emacs-real = constructs the path to Emacs-real
  4. The whole thing is wrapped in "..." (double quotes)

The double quotes around the entire "$(dirname "$0")/Emacs-real" protect against spaces and most special characters. The inner "$0" also protects the path when passed to dirname.

So are you concerned about extremely exotic cases like:

  • Paths with newlines
  • Paths with $ that might expand inside the double quotes

?

Honestly, if someone's Homebrew prefix contains shell metacharacters, everything would break, not just Emacs. So this sounds more like a theoretical concern.

Or am I missing something?

EOS
end

# Make executable
File.chmod(0755, emacs_binary)
system "touch '#{app}'"
end

Expand Down
Loading