launch middleman process on macOS to workaround SIP limit #416
Conversation
There was a problem hiding this comment.
Thanks @TingluoHuang !
This looks like it would solve our problem assuming WellKnownDirectory.Externals is outside /usr, which is true on the current actions VMs.
| // launch `node macOSRunInvoker.js shell args` instead of `shell args` to avoid macOS SIP remove `DYLD_INSERT_LIBRARIES` when launch process | ||
| string node12 = Path.Combine(HostContext.GetDirectory(WellKnownDirectory.Externals), "node12", "bin", $"node{IOUtil.ExeExtension}"); | ||
| string macOSRunInvoker = Path.Combine(HostContext.GetDirectory(WellKnownDirectory.Bin), "macOSRunInvoker.js"); | ||
| arguments = $"\"{macOSRunInvoker.Replace("\"", "\\\"")}\" \"{fileName.Replace("\"", "\\\"")}\" {arguments}"; |
There was a problem hiding this comment.
Replace("\"", "\\\"") is not enough to escape an arbitrary string for the command line. For a unix-like OS, I think you should at least also escape \ , $ and ` .
Isn't there a utility function somewhere for escaping an argument string in an OS specific way?
There was a problem hiding this comment.
we don't need to escape those since those are special character for bash/sh, the runner launch process directly, so we only need to worry about " and (space).
There was a problem hiding this comment.
Should we pass the command line using an environment variable instead? Then we avoid escaping complexity
INPUT_FILENAME AND INPUT_ARGS
And delete the env vars (delete process.env.INPUT_FILENAME etc) before launching the child process from node.
There was a problem hiding this comment.
Alright, I see you have _proc.StartInfo.UseShellExecute = false in ProcessInvoker.cs.
In that case you probably still need to worry about \ . Otherwise an argument containing \" (a backslash and a quote) might cause some trouble. After the Replace you'd end-up with \\", which I think would be interpreted as an escaped backslash and an unescaped ".
Wouldn't it be easier to use ProcessStartInfo.ArgumentList instead of ProcessStartInfo.Arguments ? That looks more convenient.
There was a problem hiding this comment.
we don't want to be in a business of parse customer's arg string into arg array. :)
TingluoHuang
force-pushed
the
users/tihuang/workaroundsip
branch
2 times, most recently
from
d9a08df to
39bc519
Compare
TingluoHuang
force-pushed
the
users/tihuang/workaroundsip
branch
from
13e78bd to
ff2b794
Compare
We do not need to prefix `$CODEQL_RUNNER` here on macOS to bypass SIP, because we assume that the `init` step exported `DYLD_INSERT_LIBRARIES` into the environment, which activates the Actions workaround for SIP. See actions/runner#416.
We do not need to prefix `$CODEQL_RUNNER` here on macOS to bypass SIP, because we assume that the `init` step exported `DYLD_INSERT_LIBRARIES` into the environment, which activates the Actions workaround for SIP. See actions/runner#416.
When
DYLD_INSERT_LIBRARIESset on macOS, SIP will remove it when launch process that under/usr/bin, ex:bashWe will use
nodeas a proxy process on macOS whenDYLD_INSERT_LIBRARIESis set to executerunsscript.More detail:
https://github.com/github/c2c-actions-runtime/issues/447