Add the ability to prevent the masking of clientId by the-coding-cuzzy · Pull Request #634 · Azure/login

Waiariki Koia (the-coding-cuzzy) · 2026-08-25T22:24:16Z

What changed

mask-client-id, a new optional boolean input defaulting to true:

- uses: azure/login@v3
  with:
    client-id: ${{ vars.AZURE_CLIENT_ID }}
    tenant-id: ${{ vars.AZURE_TENANT_ID }}
    subscription-id: ${{ vars.AZURE_SUBSCRIPTION_ID }}
    mask-client-id: false   # default: true

action.yml - declares the input with default: true
src/common/LoginConfig.ts - LoginConfig gains a maskClientId field and the existing this.mask(this.servicePrincipalId) call is now gated on it.
README.md - input table row, a mask-client-id section, and the existing client-id masking note now points at the opt-out.
__tests__/LoginConfig.test.ts - three cases covering the input set to true, set to false, and absent.

Scope and default behaviour

Existing workflows are unaffected. The input defaults to true and every current caller keeps masking the client ID.
Only the client ID/service principal ID is in scope. servicePrincipalSecret and the OIDC federated token are still masked unconditionally.
The gate applies to the client ID from either source - the client-id input or clientId inside creds - since both land in the same field.
A value passed from ${{ secrets.* }} is still masked by Actions itself regardless of this input. Opting out only has a visible effect for values that are not repository secrets, which is the case this is aimed at.

Note for reviewers

The input is parsed as "anything other than an explicit false means mask":

this.maskClientId = core.getInput('mask-client-id').toLowerCase() !== "false";

The other boolean inputs in LoginConfig use === "true", which resolves to false when the input is absent. That is fine for enable-AzPSSession and allow-no-subscriptions, but for a masking flag it fails in the unsafe direction - a missing input would silently stop masking. Inverting the comparison keeps a missing or malformed value on the masked path.

Happy to switch to core.getBooleanInput or to match the surrounding style instead if you would prefer consistency here.

Waiariki Koia (the-coding-cuzzy) · 2026-08-25T22:24:48Z

@microsoft-github-policy-service agree company="Global Dairy Trade"

MaddyMicrosoft · 2026-09-01T03:48:31Z

Waiariki Koia (@the-coding-cuzzy) Thank you for opening this PR. This change is under review with our security team. I will keep you updated.

MaddyMicrosoft

Thanks for this, and for the clear reviewer notes.

Two of your calls are correct and worth confirming:

Keep the !== "false" parse, don't switch to getBooleanInput. getBooleanInput throws on unrecognised values, which fails away from the safe default; your parse keeps a missing or malformed input on the masked path, which is the right behaviour for a masking flag.
Scoping to only the client-id (secret and federated token stay masked) is correct.

Security reviewed this and is happy to move forward. Please add a line to the mask-client-id section of the README along these lines: the client-id is effectively a username, low sensitivity on its own and only useful to an attacker who already holds the client secret or certificate, so disabling masking is reasonable when the value is treated as configuration.

Otherwise just the test fix and the small README rendering issues I've flagged inline.

Copilot

🟡 Changes recommended

A newly added unit test sets mask-client-id to true while asserting the false behavior, and the README introduces incorrect “Github” casing that should be corrected.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR adds an opt-out flag to control whether the client-id is registered as a masked secret in workflow logs, defaulting to the current (masked) behavior to preserve backward compatibility.

Changes:

Adds a new mask-client-id input (default true) to allow disabling client-id masking.
Gates core.setSecret(client-id) behind the new maskClientId flag in LoginConfig.initialize().
Updates README documentation and adds test coverage for the new input behavior.

File summaries

File	Description
`src/common/LoginConfig.ts`	Adds `maskClientId` and conditionally masks `servicePrincipalId` based on the new input.
`action.yml`	Declares the new optional `mask-client-id` input with default `true`.
`README.md`	Documents the new input and explains when it has an effect.
`__tests__/LoginConfig.test.ts`	Adds tests for `mask-client-id` set to `true`, `false`, and absent.

Review details

Files reviewed: 4/4 changed files
Comments generated: 2
Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

+        setEnv('tenant-id', 'tenant-id');
+        setEnv('subscription-id', 'subscription-id');
+        setEnv('client-id', 'client-id');
+        setEnv('mask-client-id', 'true');


…cted

Copilot

🟡 Changes recommended

The new boolean parsing should be hardened (whitespace) and the added tests should assert the actual masking side effect (core.setSecret) to prevent regressions.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (2)

tests/LoginConfig.test.ts:198

This test should also verify that opting out (mask-client-id=false) actually prevents masking, i.e., core.setSecret is not called.

    test('initialize with mask-client-id=false', async () => {
        setEnv('environment', 'azureusgovernment');
        setEnv('enable-AzPSSession', 'false');
        setEnv('allow-no-subscriptions', 'true');
        setEnv('auth-type', 'SERVICE_PRINCIPAL');

tests/LoginConfig.test.ts:214

When mask-client-id is omitted, the key behavior is that masking still happens by default. This test currently only checks the boolean, so it won’t catch regressions where masking is accidentally skipped.

    test('initialize without mask-client-id', async () => {
        setEnv('environment', 'azureusgovernment');
        setEnv('enable-AzPSSession', 'false');
        setEnv('allow-no-subscriptions', 'true');
        setEnv('auth-type', 'SERVICE_PRINCIPAL');

Files reviewed: 4/4 changed files
Comments generated: 2
Review effort level: Lite

        this.federatedToken = null;

-        this.mask(this.servicePrincipalId);
+        this.maskClientId = core.getInput('mask-client-id').toLowerCase() !== "false";


+    test('initialize with mask-client-id=true', async () => {
+        setEnv('environment', 'azureusgovernment');
+        setEnv('enable-AzPSSession', 'false');
+        setEnv('allow-no-subscriptions', 'true');
+        setEnv('auth-type', 'SERVICE_PRINCIPAL');
+        setEnv('tenant-id', 'tenant-id');
+        setEnv('subscription-id', 'subscription-id');
+        setEnv('client-id', 'client-id');
+        setEnv('mask-client-id', 'true');
+
+        let loginConfig = new LoginConfig();
+        await loginConfig.initialize();
+        expect(loginConfig.maskClientId).toBeTruthy();
+        expect(loginConfig.servicePrincipalId).toBe("client-id");
+    });
+


Add the ability to prevent the masking of clientId

b2ae28c

MaddyMicrosoft self-assigned this Sep 1, 2026

MaddyMicrosoft requested changes Sep 4, 2026

View reviewed changes

Waiariki Koia (the-coding-cuzzy) added 2 commits September 4, 2026 16:37

Fix formatting in README.md and tests

4ba51e0

Add paragraph to README about client-id being low sensitivity on its own

7bd0703

Teresa Ritorto (teresaritorto) requested a lite review from Copilot September 7, 2026 00:36

Copilot started reviewing on behalf of Teresa Ritorto (teresaritorto) September 7, 2026 00:37 View session

Copilot AI reviewed Sep 7, 2026

View reviewed changes

Comment thread __tests__/LoginConfig.test.ts Outdated

setEnv('tenant-id', 'tenant-id');

setEnv('subscription-id', 'subscription-id');

setEnv('client-id', 'client-id');

setEnv('mask-client-id', 'true');

Comment thread README.md Outdated

Waiariki Koia (the-coding-cuzzy) added 2 commits September 7, 2026 20:19

Change Github to GitHub for consistent casing

7293f80

Give correct false input to test mask-client-id=false works as expe…

f9226c3

…cted

Waiariki Koia (the-coding-cuzzy) requested a lite review from Copilot September 7, 2026 08:22

Copilot started reviewing on behalf of Waiariki Koia (the-coding-cuzzy) September 7, 2026 08:23 View session

Copilot AI reviewed Sep 7, 2026

View reviewed changes

Navigation Menu

Uh oh!

Add the ability to prevent the masking of clientId - #634

Add the ability to prevent the masking of clientId#634
Waiariki Koia (the-coding-cuzzy) wants to merge 5 commits into
Azure:masterfrom
the-coding-cuzzy:not-mask-client-id

Waiariki Koia (the-coding-cuzzy) commented Aug 25, 2026

Uh oh!

Waiariki Koia (the-coding-cuzzy) commented Aug 25, 2026 •

edited

Loading

Uh oh!

MaddyMicrosoft commented Sep 1, 2026

Uh oh!

MaddyMicrosoft left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

Waiariki Koia (the-coding-cuzzy) commented Aug 25, 2026

What changed

Scope and default behaviour

Note for reviewers

Uh oh!

Waiariki Koia (the-coding-cuzzy) commented Aug 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MaddyMicrosoft commented Sep 1, 2026

Uh oh!

MaddyMicrosoft left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Suppressed comments (2)

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Waiariki Koia (the-coding-cuzzy) commented Aug 25, 2026 •

edited

Loading