Skip to content

fix: address SFI security compliance issues#989

Open
VishalS-Microsoft wants to merge 7 commits into
dev-v4from
psl-fixexpsfiissues-macae
Open

fix: address SFI security compliance issues#989
VishalS-Microsoft wants to merge 7 commits into
dev-v4from
psl-fixexpsfiissues-macae

Conversation

@VishalS-Microsoft
Copy link
Copy Markdown
Contributor

Purpose

This pull request updates data collection rules and storage account settings in the infrastructure Bicep and JSON templates, with a focus on improving security and event collection capabilities. The most significant changes include enhancements to Windows event data collection, the addition of infrastructure encryption for storage accounts, and updates to generator metadata.

Windows Event Data Collection Enhancements:

  • Changed the event stream from 'Microsoft-WindowsEvent' to 'Microsoft-Event' and updated the xPathQueries filter to target events with specific keywords while excluding EventID 4624, allowing for more precise audit event collection. (infra/main.bicep, infra/main_custom.bicep, infra/main.json) [1] [2] [3]
  • Added a new output stream for 'Microsoft-Event' with corresponding destinations and KQL transformation, enabling additional event data routing. (infra/main.bicep, infra/main_custom.bicep, infra/main.json) [1] [2] [3]

Storage Account Security:

  • Enabled infrastructure encryption for storage accounts by setting requireInfrastructureEncryption: true, increasing data-at-rest security. (infra/main.bicep, infra/main_custom.bicep, infra/main.json) [1] [2] [3]

Generator Metadata and Template Maintenance:

  • Updated Bicep generator version and template hashes throughout the JSON templates to reflect the latest build and ensure consistency. (infra/main.json) [1] [2] [3] [4] [5] [6] [7]

Dependency Ordering:

  • Adjusted the order of dependencies for private DNS zones to ensure correct deployment sequencing. (infra/main.json)

These changes collectively improve the security posture and event monitoring capabilities of the deployed infrastructure.

Does this introduce a breaking change?

  • Yes
  • No

How to Test

  • Get the code
git clone [repo-address]
cd [repo-name]
git checkout [branch-name]
npm install
  • Test the code

What to Check

Verify that the following are valid

  • ...

Other Information

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the infrastructure templates to meet SFI security compliance needs by tightening storage account encryption settings and refining Windows VM audit event collection via Azure Monitor Data Collection Rules (DCRs).

Changes:

  • Updated Windows Security event collection to use the Microsoft-Event stream and a more targeted xPathQueries filter, plus added a corresponding dataFlows entry.
  • Enabled Storage Account infrastructure encryption (requireInfrastructureEncryption: true).
  • Refreshed generated ARM template metadata (Bicep generator version/template hashes) and adjusted a dependency ordering entry in main.json.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
infra/main.bicep Updates Windows VM DCR event stream/query and enables infrastructure encryption on the storage account.
infra/main_custom.bicep Mirrors the same DCR and storage encryption changes for the custom template variant.
infra/main.json Regenerated ARM template reflecting the above changes (DCR stream/query + new dataFlow, storage infrastructure encryption, metadata/hash updates, and dependency ordering).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread infra/main.bicep
Comment thread infra/main_custom.bicep
Copy link
Copy Markdown
Contributor

@Harsh-Microsoft Harsh-Microsoft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copilot AI review requested due to automatic review settings May 19, 2026 15:58
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

Comment thread infra/main.json Outdated
Comment thread infra/main.json Outdated
Copilot AI review requested due to automatic review settings May 20, 2026 10:11
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (6)

infra/main.bicep:1221

  • ingressAllowInsecure is set to false (HTTPS-only), but corsPolicy.allowedOrigins still includes an http://... origin. This weakens the security posture and can trip compliance checks; consider removing the HTTP origin (and regenerate infra/main.json after updating the Bicep).

This issue also appears on line 1425 of the same file.

    // SFI: Enforce HTTPS-only ingress. When false, HTTP requests are automatically redirected to HTTPS.
    ingressAllowInsecure: false
    corsPolicy: {
      allowedOrigins: [
        'https://${webSiteResourceName}.azurewebsites.net'
        'http://${webSiteResourceName}.azurewebsites.net'
      ]

infra/main.bicep:1431

  • ingressAllowInsecure is set to false (HTTPS-only), but corsPolicy.allowedOrigins still includes an http://... origin. Consider removing the HTTP origin to align CORS with HTTPS-only ingress (and regenerate infra/main.json after updating the Bicep).
    // SFI: Enforce HTTPS-only ingress. When false, HTTP requests are automatically redirected to HTTPS.
    ingressAllowInsecure: false
    corsPolicy: {
      allowedOrigins: [
        'https://${webSiteResourceName}.azurewebsites.net'
        'http://${webSiteResourceName}.azurewebsites.net'
      ]

infra/main_custom.bicep:1248

  • ingressAllowInsecure is set to false (HTTPS-only), but corsPolicy.allowedOrigins still includes an http://... origin. Consider removing the HTTP origin to align CORS with HTTPS-only ingress (and regenerate infra/main.json after updating the Bicep).

This issue also appears on line 1467 of the same file.

    // SFI: Enforce HTTPS-only ingress. When false, HTTP requests are automatically redirected to HTTPS.
    ingressAllowInsecure: false
    corsPolicy: {
      allowedOrigins: [
        'https://${webSiteResourceName}.azurewebsites.net'
        'http://${webSiteResourceName}.azurewebsites.net'
      ]

infra/main_custom.bicep:1473

  • ingressAllowInsecure is set to false (HTTPS-only), but corsPolicy.allowedOrigins still includes an http://... origin. Consider removing the HTTP origin to align CORS with HTTPS-only ingress (and regenerate infra/main.json after updating the Bicep).
    // SFI: Enforce HTTPS-only ingress. When false, HTTP requests are automatically redirected to HTTPS.
    ingressAllowInsecure: false
    corsPolicy: {
      allowedOrigins: [
        'https://${webSiteResourceName}.azurewebsites.net'
        'http://${webSiteResourceName}.azurewebsites.net'
      ]

infra/main.json:38458

  • ingressAllowInsecure is false (HTTPS-only), but the CORS allowedOrigins list still contains an http://... origin. For a stricter security posture/compliance alignment, remove the HTTP origin so only HTTPS origins are permitted.

This issue also appears on line 40195 of the same file.
infra/main.json:40203

  • ingressAllowInsecure is false (HTTPS-only), but the CORS allowedOrigins list still contains an http://... origin. Consider removing the HTTP origin so CORS matches the HTTPS-only ingress setting.

Comment thread infra/main.json
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants