client-id/serviceprincipal id incorrectly masked

[bt701]

This makes logs a bit more difficult and confusing to read.

login/src/common/LoginConfig.ts

Line 66 in bbcc074

this.servicePrincipalId = this.servicePrincipalId ? this.servicePrincipalId : secrets.clientId;

login/src/common/LoginConfig.ts

Line 45 in bbcc074

this.mask(this.servicePrincipalId);

Usually I would expect client-ids to be public information, but this action treats them as a secret.
If there is a reason, perhaps that needs to be documented?

[YanaXu]

Hi @bt701,

Thank you for your question.
Indeed, some users consider the client ID to be public information; however, many others view it as private. These users are primarily from enterprise environments, where the team responsible for configuring the resources required by the login action is different from the team that actually uses the token after login. Since these teams have different permission scopes for resource management, they do not want the client ID to be treated as public information and passed through to all repository users, as this could introduce certain security risks.
It is also worth noting that secrets in GitHub Actions are not necessarily fully secure. Any workflow with read access is technically capable of retrieving their values and printing them. We classify them as secrets primarily to prevent users from accidentally logging their values, which could otherwise cause issues—especially when the GitHub Action is used in a public repository.
We hope this explanation addresses your concerns.

[bt701]

Hey @YanaXu thanks for the reply.

It's good doc to have this represented in the issue so I do appreciate it.
Any organization that views a public piece of data as sensitive is terrifying, and likely have a fundamental misunderstanding of basic security, so hopefully they look at this issue and mend their ways :)

As is, it does violate the principal of least surprise. As I discovered it from other log lines, as GitHub actions marks the occurrence of the secret globally. However at least there is now some documentation that it is a conscious decision.

I'd still vote to remove it, but I truly appreciate your response.