POSIX Warning on JSON Secrets

[aaronfagan]

Hello!

I found that when using JSON-ified secrets parsing, I receive a warning in GitHub Actions outlining the name is not POSIX compliant. While I appreciate the message, Im wondering if there is an absolute reason for this output?

It seems to be a bit of an anti-pattern and a potential security risk to output an easily readable map of every secrets that is saved.

Perhaps this messaging can be removed on these ground?

Thanks!

[aaronfagan]

I would love to submit a PR - can't figure out how tho!

As far as I can tell, the only change is to remove this block:

    if (secretName !== secretNamePOSIX) {
      const part1 = `Secret name '${secretName}' is not POSIX compliant. `
      const part2 = `It will be changed to '${secretNamePOSIX}'.\n\n`
      const part3 = 'POSIX compliance: environment variable names can only contain upper case letters, '
      const part4 = 'digits and underscores. It cannot begin with a digit.'
      core.warning(part1.concat(part2).concat(part3).concat(part4))
    }

[aaronfagan]

Perhaps a debug variable that must be true to provide this and other warnings?

[abhilash1in]

@aaronfagan you will see this warning if your secret name doesn't meet these conditions: can only contain upper case letters, digits and underscores. It cannot begin with a digit..

If a secret name which violates these conditions is used as-is for environment variable name, then bash will not recognize this environment variable. Therefore, the name is transformed into something bash will recognize.

The warning is crucial since if the secret name doesn't meet the above conditions, injected environment variable name will not be same as secret name. The user may not be expecting this.

What's the name of the secret you're fetching? Before posting it here, feel free to mask lowercase characters with x, uppercase characters with X, digits with D or something similar if you don't want to reveal the exact name.

[abhilash1in]

It seems to be a bit of an anti-pattern and a potential security risk to output an easily readable map of every secrets that is saved.

Can you elaborate on this? The logs only contain secret names but not values. Is that still a security risk in your case?

Moreover, If you make the secret names (on AWS) compliant with the above conditions, you should not see these warnings.

Without making them compliant on AWS or via this action, there's no way to inject them as environment variables. If we forcefully make them compliant, the warning you mentioned is crucial to indicate this transformation.

[abhilash1in]

Perhaps a debug variable that must be true to provide this and other warnings?

There are other debug logs already, which you will see if you enable step debug logging.

[aaronfagan]

Hi @abhilash1in !

Here is the structure and usecase I am facing:

I have a secret in AWS Secrets Manager containing a JSON object of multiple secrets. Lets call this secret client1/common (this follows my actual naming convention). It contains a JSON object that looks like this:

{
     "secret1": "value1",
     "secret2": "value2",
     "secret3": "value3",
     "secret4": "value4"
}

I am parsing the JSON. Im currently only using secret1 and secret2 values, however, I want all secrets available in the event I use them at a later time.

Per the docs, these will be replaced and named $CLIENT1_COMMON_SECRET1 and $CLIENT1_COMMON_SECRET2. However, due to the warning output, anyone viewing my job is now aware that $CLIENT1_COMMON_SECRET3 and $CLIENT1_COMMON_SECRET4 exist within my secret. This is against the concept of not divulging additional information about your infrastructure unless it is absolutely necessary.

I understand the want to inform users that their environment variable name may be different than expected. However, given the above concern, this may make more sense to simply add additional documentation around how secrets are transformed into environment variables names.

Thanks!

PS - Great Action by the way!

[abhilash1in]

@aaronfagan I've addressed this in 83b2355d5e494f9b1f7af6ac8d914ba47bd6bed8.

The warning message is now generic and doesn't include actual variable names. Actual variable names are only logged in debug mode.

This should in the next release. Please feel free to re-open the issue if it doesn't solve your concerns.