Runner corrupts secrets containing backslashes

[redlizard]

As of version 2.296.0, if the actions runner creates a docker container to execute an action which receives github secrets as environment variables, any backslash characters in the secret values get doubled. This results in the docker container receiving an incorrect value of the secret.

Commit 01fd044 escapes all command line arguments to docker commands by replacing double-quote and backslash character with an escape sequence, which includes github secrets. This would work on unix, but the action runner dotnet framework uses the windows-native escape scheme, in which \\ is not an escape sequence in most conditions. Thus, escaped backslashes in secret values are not interpreted correctly when starting a docker process using the resulting command line arguments.

[thboop]

Hey @redlizard thank you for the report we are digging into this issue.

Can you confirm if you are seeing this in container actions or just job/service containers.

Do you have an example workflow yaml or action yaml you can provide?

[redlizard]

I am indeed seeing this in container actions, not service containers. I do not have a prepared testcase, but I could write one if necessary.

I do not think your solution posted here is quite correct. According to this specification, which I think also applies to the dotnet process-spawning procedures, \\ is an escape sequence as long as it directly preceeds a quote character; your solution does not implement this, and therefore reopens the problem fixed in #2062. Specifically, that means that in any sequence of backslash characters followed by a double-quote character, the backslashes act as escape characters; whereas in any other context, they are regular characters.

I have not checked that this specification does in fact apply to the process creation call used by the actions runner; it is only my best guess based on observed behavior so car. Caveat lector.

[nikola-jokic]

Hey @redlizard,

Please provide one testcase, so we can better understand the issue. This PR aims to solve this issue when passing environment variables to the docker. Please, let us know if that solves your issue ☺️

[redlizard]

I think #2091 solved it, and #2108 solved it better. I have not yet had the opportunity to test either of them, but it looks good to me on first glance.

[nikola-jokic]

Thank you, @redlizard, I will close this issue now but feel free to re-open it or submit another one if you see any problems with this ☺️