Registries Proxy: Support feeding a base64 encoded configuration #2404
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
src/start-proxy-action.ts
Outdated
| function getCredentials(): Credential[] { | ||
| const encodedCredentials = actionsUtil.getOptionalInput("registries_credentials"); | ||
| if (encodedCredentials !== undefined) { | ||
| const credentialsStr = Buffer.from(encodedCredentials, "base64").toString(); |
There was a problem hiding this comment.
This seems like some extra hoops to go through for the workflow file to send base64 encoded credentials. Does it make more sense to accept non-encoded credentials and only base64 encode them in the action?
There was a problem hiding this comment.
Indeed, there are some extra steps, but I think they are justified.
The proxy takes a JSON object as previously defined in registry_secrets. The problem is that Actions recommends not using structured data within secrets:
To help ensure that GitHub redacts your secret in logs, avoid using structured data as the values of secrets.
Therefore, we encode it as a single string to make scrubbing possible.
The underlying reason we need a struct is that (due to the integration with default setup) we need to provide a potentially arbitrary number of credentials, and we cannot easily know which ones/how many beforehand.
marcogario
force-pushed
the
marcogario/proxy_64
branch
3 times, most recently
from
7156568 to
a9c1d1c
Compare
marcogario
force-pushed
the
marcogario/proxy_64
branch
from
a9c1d1c to
0b84d89
Compare
|
@aeisenberg this is ready for another review. I am also assigning @aibaars so he can shepherd this to completion in case there are issues to address. |
Extend the start-proxy action to support passing the configuration as a base64 encoded variable. The reason to do so, is that this information is being passed by Actions as a secret. Actions recommends against passing structured data as a secret because it becomes harder to scrub it. Therefore, we send the information as a simple string.
We introduce the new input
registries_credentialsin parallel with the previous input to make it easier to test this while we are still building the remaining parts of the system. We will deprecate and remove the old property (registry_secrets) in the future. Note that this entire action is meant to be for internal use only.Finally, in this PR I also refactored a bit the logic to more clearly distinguish the key steps, perform some validation of the input, and thread the logger for increase observability.
Merge / deployment checklist