chore(m365): use PowerShell best practices for quoting credential variables

[sandiyochristan]

Description

Adopt PowerShell quoting best practices for credential variables in the M365 provider's PowerShell session. Literal values such as the client ID, tenant ID and client secret are now assigned with single-quoted strings instead of double-quoted ones, which is the recommended convention for literals in PowerShell: single quotes treat the content verbatim and avoid unintended variable interpolation or subexpression evaluation. Embedded single quotes are escaped following the standard ' to '' rule.

This is a general robustness and consistency improvement to how credentials are handled in the PowerShell layer, aligning the Application Auth path with the Certificate Auth path.

Checklist

• I have read the CONTRIBUTING.md
• I have verified the changes locally

[github-actions]

✅ Conflict Markers Resolved

All conflict markers have been successfully resolved in this pull request.

[danibarranqueroo]

Hello @sandiyochristan!

Thanks for this contribution, we'll review and test it once we have time 🙌

[HugoPBrito]

Hey @sandiyochristan, thanks for the contribution!

The core idea here is solid. You're right that double quotes in PowerShell expand variables and that's a real problem for secrets. Let me break down what we'd need to land this.

✅ What works (with small tweaks)

1. client_secret → single quotes (line 139): This is the real win of the PR. A legitimate secret like Pa$$w0rd silently gets corrupted today because PowerShell expands $$. Single quotes with the ' → '' escape is the correct PowerShell convention. One important note: don't use self.sanitize() on the secret — its regex strips chars like $, !, # that are perfectly valid in secrets.

2. client_id / tenant_id → add self.sanitize() in Application Auth (lines 138-140): The Certificate Auth path already does this (lines 118-119), Application Auth should be consistent. These are UUIDs so sanitize() is safe here. Switch to single quotes too for uniformity.

3. Remove double quotes around $clientSecret in test_exchange_connection (line 199). It's already a PS variable at that point, wrapping it in "..." is redundant and could re-expand special chars that were correctly handled during assignment.

❌ Please remove

The security-best-practices.mdx doc — We appreciate the thought, but it's generic content that doesn't follow the project's documentation style. Let's keep this PR focused on the actual code fix.

📝 What's missing to merge

• Update existing tests — test_init_credential assertions still expect the old double-quote format, they'll fail with these changes.
• Add a test for special chars — Something like a secret with Pa$$w0rd!#' test to prove the escape logic actually works. That's the whole point of this fix, we should have a test covering it.

A note on the framing

The PR describes this as a command injection fix, that's overstated. Before any credential reaches PowerShell, validate_static_credentials() authenticates against Azure AD via MSAL. A malicious secret would fail auth and never get here.

The real value of this change is correctness: preventing silent corruption of legitimate secrets that happen to contain special characters. That's still worth fixing, just want to set the right expectations.

Let us know if you have questions, happy to help with the test cases!

[sandiyochristan]

Thanks for the detailed review @HugoPBrito! All feedback addressed in the latest commit:

Done:

• ✅ Removed security-best-practices.mdx
• ✅ client_secret → single quotes with ' → '' escape (no sanitize() — preserves $, !, # etc.)
• ✅ client_id / tenant_id → added self.sanitize() in Application Auth path (consistent with Certificate Auth)
• ✅ Switched client_id / tenant_id to single quotes for uniformity
• ✅ Removed double quotes around $clientSecret in test_exchange_connection (was already done in prior commit)
• ✅ Updated test_init_credential assertions to match new single-quote format
• ✅ Added test_init_credential_special_chars_in_secret — tests a secret with Pa$$w0rd!#' to prove the escape logic preserves $, !, # and correctly escapes ' as ''

Re: framing — Agreed, this is a correctness fix (preventing silent corruption of legitimate secrets with special chars), not a command injection fix. Updated the PR title accordingly.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.89%. Comparing base (7d03bc5) to head (edce93e).
⚠️ Report is 4 commits behind head on master.

❗ There is a different number of reports uploaded between BASE (7d03bc5) and HEAD (edce93e). Click for more details.

HEAD has 1 upload less than BASE

Flag	BASE (7d03bc5)	HEAD (edce93e)
api	1	0

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #9997      +/-   ##
==========================================
- Coverage   93.97%   87.89%   -6.08%     
==========================================
  Files         237      133     -104     
  Lines       34829     5724   -29105     
==========================================
- Hits        32729     5031   -27698     
+ Misses       2100      693    -1407     

Flag	Coverage Δ
api	?
prowler-py3.10-m365	87.89% <100.00%> (?)
prowler-py3.11-m365	87.36% <100.00%> (?)
prowler-py3.12-m365	87.89% <100.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Components	Coverage Δ
prowler	87.89% <100.00%> (∅)
api	∅ <ø> (∅)

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[HugoPBrito]

Thanks for the updates. I only see two small follow-ups left:

• Please strengthen the Exchange test coverage by asserting the exact command used for the secure secret conversion, especially
  ConvertTo-SecureString $clientSecret -AsPlainText -Force, so we catch any regression back to "$clientSecret".
• Please update the init_credential() docstring/note to match the current behavior. client_id and tenant_id are sanitized,
  but client_secret is intentionally not sanitized anymore and is only single-quote escaped to preserve valid special characters.

After that I'll functionally test the PR in depth to ensure it's working as expected.

[HugoPBrito]

Pushed the follow-ups and verified the fix.

• Docstring fixed: client_id / tenant_id sanitized, client_secret only single quote escaped.
• Tests (47 passing): exact ConvertTo-SecureString $clientSecret assertion + edge cases ($, $(...), quotes, injection payload, empty, None).
• Verified in PowerShell 7.5.2 with secret Pa$$w0rd!#'$(hostname)-Xyz: the new code preserves it identical, byte for byte, with no expansion or subexpression execution.

[HugoPBrito]

Reframed this PR as a best practices improvement rather than a security fix.

What changed: the M365 PowerShell session now assigns credential variables (client ID, tenant ID, client secret) using single-quoted strings, the recommended PowerShell convention for literal values. Single quotes treat the content verbatim, so there is no variable interpolation or subexpression evaluation, and embedded single quotes are escaped as ''.

On scope: Microsoft auto-generated Entra client secrets only use the character set [A-Za-z0-9._~-] (per the Microsoft Learn definition linked below), none of which are affected by double-quoted PowerShell strings, so this does not fix a reachable bug for standard credentials. We still validated the quoting behavior in depth for completeness: a parametrized unit suite plus a live check in PowerShell 7.5.2 confirm that values containing characters like $, $(...), backticks and quotes are preserved verbatim through the SecureString conversion. The change keeps the Application Auth path consistent with the Certificate Auth path and follows PowerShell best practices.

Reference: https://learn.microsoft.com/en-us/purview/sit-defn-azure-ad-client-secret