fix: XML-escape dynamic values in launchd plist (#181) - #206
Open
leanworld7-netizen wants to merge 1 commit into
Open
fix: XML-escape dynamic values in launchd plist (#181)#206leanworld7-netizen wants to merge 1 commit into
leanworld7-netizen wants to merge 1 commit into
Conversation
The macOS start_with_launchd() function writes dynamic values ($agent_path, $auth_url, $auth_client_id, $auth_audience, $autospin_url, $MONK_AGENT_LOCAL, $MONK_PLUGIN_VERSION, $host, $agent_path_env) directly into the launchd plist XML using a heredoc without XML escaping. If any of these values contain XML-special characters (&, <, >, ", '), the resulting plist is malformed XML that launchd silently rejects — the agent never starts and no error is surfaced. This adds an xml_escape() shell function that performs the five required XML entity replacements and applies it to all dynamic values before they are interpolated into the plist heredoc. The launchd_configured() grep checks are also updated to match against the escaped values, ensuring the idempotency check correctly detects an already-configured plist. Fixes monk-io#181
Author
|
Linking to bounty issue #181. This fix addresses the reported vulnerability. Ready for review. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The macOS
start_with_launchd()function writes dynamic values ($agent_path,$auth_url,$auth_client_id,$auth_audience,$autospin_url,$MONK_AGENT_LOCAL,$MONK_PLUGIN_VERSION,$host,$agent_path_env) directly into the launchd plist XML using a heredoc without XML escaping.If any of these values contain XML-special characters (
&,<,>,",'), the resulting plist is malformed XML that launchd silently rejects — the agent never starts and no error is surfaced to the user.Fix
Adds an
xml_escape()shell function that performs the five required XML entity replacements (&→&,<→<,>→>,"→",'→') and applies it to all dynamic values before they are interpolated into the plist heredoc.The
launchd_configured()grep checks are also updated to match against the escaped values, ensuring the idempotency check correctly detects an already-configured plist.Testing
&(e.g.,/tmp/agent&fixture/monk-agent) are properly escaped to/tmp/agent&fixture/monk-agentin the plist XMLFixes #181
Bounty eligibility: I have signed up at monk.io and used the product.