Skip to content

fix(infra): address SFI security compliance issues#244

Merged
Prajwal-Microsoft merged 10 commits into
devfrom
psl-sw/43311-sfi-security-fixes
May 22, 2026
Merged

fix(infra): address SFI security compliance issues#244
Prajwal-Microsoft merged 10 commits into
devfrom
psl-sw/43311-sfi-security-fixes

Conversation

@Shreyas-Microsoft
Copy link
Copy Markdown
Contributor

@Shreyas-Microsoft Shreyas-Microsoft commented May 18, 2026

Purpose

Addresses ADO #43311Container Migration - Check and Fix SFI Issues reported by EXP Team.

The EXP team flagged five SFI (Secure Future Initiative) items on this accelerator. Each is addressed in the infra layer (Bicep + the generated ARM template). No application code is touched. Pattern reference: microsoft/Modernize-your-code-solution-accelerator#435.

SFI items addressed

# SFI item Where
1 endToEndEncryptionEnabled: true for App Service ↔ Container Apps equivalent (peerTrafficEncryption / mTLS for intra-environment traffic) infra/main.bicep, infra/main_custom.bicep — Container Apps Environment
2 Data Collection Rule capturing all Windows Security audit success / audit failure events (Keywords bitmask 0x30000000000000) except EventID 4624 (very high-volume successful logon), streamed via Microsoft-SecurityEvent to Log Analytics, plus a small Microsoft-Perf set (CPU / memory / disk / network) on the same DCR for jumpbox VM health, associated with the jumpbox VM infra/main.bicep, infra/main_custom.bicep
3 disableLocalAuth: true for AI Services Already in dev via the AVM cognitive-services/account:0.13.2 module on aiFoundryAiServices; app code uses Entra ID (use_entra_id=True) everywhere
4 identity: { type: 'SystemAssigned' } where missing Cosmos DB, Azure Container Registry, Jumpbox VM in infra/main.bicep, infra/main_custom.bicep (resources already on UAMI were not modified)
5 encryption.requireInfrastructureEncryption: true set explicitly (rather than relying on the AVM default) infra/modules/storageAccount.bicep

Does this introduce a breaking change?

  • Yes
  • No

Infra-only. Identity additions are additive; peerTrafficEncryption is transparent to workloads; the DCR is a new resource with no impact on existing ones; requireInfrastructureEncryption matches the existing AVM default.

Golden Path Validation

  • Existing backend + processor unit suites continue to pass and meet the 82% coverage gate. No app code touched.

Deployment Validation

  • az bicep build infra/main.bicep and az bicep build infra/main_custom.bicep succeed with no new warnings/errors (only the pre-existing BCP334 warning on main_custom.bicep, unchanged by this PR).
  • infra/main.json regenerated from main.bicep (not hand-edited).

What to Check

Verify that the following are valid:

  • peerTrafficEncryption: true is set on the Container Apps Environment in infra/main.bicep and infra/main_custom.bicep and intra-environment traffic between revisions is encrypted (mTLS) on a deployed resource group.
  • The Microsoft.Insights/dataCollectionRules resource dcr-<solutionSuffix> is created (gated on enablePrivateNetworking && enableMonitoring), is associated with the jumpbox VM, and that SecurityEvent rows for audit failures (e.g. EventID == 4625) appear in the Log Analytics workspace after a failed logon on the jumpbox. Successful-logon EventID 4624 rows are intentionally excluded by the xPath filter Security!*[System[(band(Keywords,13510798882111488)) and (EventID != 4624)]] to control ingestion cost while still satisfying the SFI audit-success/audit-failure requirement via the other audit events (4625, 4634, 4647, 4672, etc.). The SecurityEvent table is auto-provisioned by Azure Monitor on first ingestion via the DCR — no OMSGallery/Security solution is required (removed in commit e9215d2).
  • The same DCR also streams Microsoft-Perf for the jumpbox VM (CPU, memory, disk, network) and rows appear in the Perf table in the Log Analytics workspace shortly after the jumpbox starts.
  • Cosmos DB, ACR, and the Jumpbox VM each show a system-assigned managed identity in the portal after deployment, and pre-existing resources that already used a user-assigned identity are unchanged.
  • The storage account is created with requireInfrastructureEncryption: true (double encryption at rest) and existing storage-dependent workflows continue to read/write blobs and queues.
  • aiFoundryAiServices deploys with disableLocalAuth: true and app code paths that talk to AI Services (src/backend-api/, src/processor/) continue to succeed using Entra ID — no key-based fallback is triggered.

Other Information

Commits

  1. b410134 — fix(infra): require infrastructure encryption on storage accounts
  2. 8d2391f — fix(infra): enable peer traffic encryption on container apps environment
  3. 817d529 — fix(infra): add system-assigned identity to cosmos, ACR, and jumpbox VM
  4. ffc1eed — fix(infra): add windows security audit DCR + OMS Security solution
  5. 786b7e2 — chore(infra): regenerate main.json from main.bicep
  6. e9215d2 — fix(infra): remove legacy OMSGallery/Security solution (the DCR alone, via the Microsoft-SecurityEvent stream and the modern AMA pipeline, auto-provisions the SecurityEvent table; the OMSGallery/Security solution is a Log Analytics Agent / MMA-era artifact and is not required)
  7. 74a2a63 — fix(infra): apply DCR review feedback from PR fix(infra): address SFI security compliance issues #244 — broaden the xPath filter to the audit Keywords bitmask (0x30000000000000) excluding 4624; add a Microsoft-Perf data source (CPU / memory / disk / network) and matching data flow; extract the destination name to var dcrLogAnalyticsDestinationName = 'la-<lawName>-destination' and use it in both destinations.logAnalytics and both dataFlows entries
  8. 873d6e5 — fix(infra): expand DCR counterSpecifiers to the 46-counter pattern established in Multi-Agent-Custom-Automation-Engine-Solution-Accelerator PR #989 (detailed Processor / System / Process / Memory / LogicalDisk / Network Interface counters) and rename the data source to perfCounterDataSource60 for consistency with that pattern
  9. 062f656 — fix(infra): align the jumpbox VM's extensionMonitoringAgentConfig gate to the same (enablePrivateNetworking && enableMonitoring) expression used for the windowsVmDataCollectionRules module (so the windowsVmDataCollectionRules!.outputs.resourceId dereference stays safe regardless of the outer jumpbox VM gate); reword the adjacent comment to drop the misleading (4624) mention; remove all (ADO #43311) references from SFI comments in infra/main.bicep, infra/main_custom.bicep, and infra/modules/storageAccount.bicep

Verification

  • Backend unit tests: 585 passed, 93.28% coverage (gate: 82%)
  • Processor unit tests: 812 passed, 87.44% coverage (gate: 82%)
  • az bicep build infra/main.bicep — clean
  • az bicep build infra/main_custom.bicep — clean (only the pre-existing BCP334 warning, unchanged)

Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com

Shreyas-Microsoft and others added 5 commits May 18, 2026 18:25
…O #43311)

Enables double encryption at rest by setting requireInfrastructureEncryption: true
on the AVM storage-account modules used by both the standard and custom deployments,
plus the (currently unreferenced) wrapper module for parity with Modernize PR #435.

Files touched:
- infra/main.bicep                    (inline AVM storage/storage-account:0.20.0)
- infra/main_custom.bicep             (inline AVM storage/storage-account:0.20.0)
- infra/modules/storageAccount.bicep  (wrapper around AVM 0.26.2)

Addresses SFI item: "add encryption property and make requireInfrastructureEncryption: true
for storage account". Mirrors the storage-account change in
microsoft/Modernize-your-code-solution-accelerator#435.

Work item: AB#43311
ADO: https://dev.azure.com/CSACTOSOL/CSA%20Solutioning/_workitems/edit/43311

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ent (ADO #43311)

Sets peerTrafficEncryption: true on the AVM app/managed-environment:0.11.2 module
in both deployment variants. This toggles
Microsoft.App/managedEnvironments.properties.peerTrafficConfiguration.encryption.enabled,
which is the Container Apps equivalent of the App Service endToEndEncryptionEnabled
property called out by the SFI scan (this repo deploys Container Apps, not App Service).

Files touched:
- infra/main.bicep         (containerAppsEnvironment module ~L1121)
- infra/main_custom.bicep  (containerAppsEnvironment module ~L1074)

Addresses SFI item: "endToEndEncryptionEnabled: true in App Service".

Work item: AB#43311
ADO: https://dev.azure.com/CSACTOSOL/CSA%20Solutioning/_workitems/edit/43311

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…VM (ADO #43311)

Audited every resource in the Bicep templates against the SFI "identity required"
rule. Resources that support managed identity but were missing one:

  * Microsoft.DocumentDB/databaseAccounts (cosmosDb)             -> add SystemAssigned
  * Microsoft.ContainerRegistry/registries (containerRegistry)   -> add SystemAssigned
  * Microsoft.Compute/virtualMachines (jumpboxVM)                -> add SystemAssigned

The jumpbox VM also gains SystemAssigned because the Azure Monitor Agent extension
needs an identity to authenticate against the Log Analytics workspace when honoring
the SecurityAuditEvents data collection rule association (introduced in a later
commit on this branch).

Resources already compliant and left untouched:
  * aiFoundryAiServices  -> systemAssigned + userAssignedResourceIds already set
  * aiFoundryProject     -> identity.type = 'SystemAssigned' already set
  * appConfiguration / avmAppConfigUpdated -> systemAssigned already set
  * containerAppsEnvironment -> systemAssigned already set
  * containerAppBackend / Frontend / Processor -> UAMI (appIdentity) already wired
  * storageAccount       -> systemAssigned already set
  * appIdentity (UAMI itself, N/A)
  * Bastion / ApplicationInsights / LAW / PrivateDnsZones (do not support / not in
    SFI scope per user's authoritative list)

Files touched:
- infra/main.bicep         (cosmosDb, jumpboxVM)
- infra/main_custom.bicep  (cosmosDb, containerRegistry, jumpboxVM)

Addresses SFI item: "identity: { type: 'SystemAssigned' } or { type: 'UserAssigned' }".

Work item: AB#43311
ADO: https://dev.azure.com/CSACTOSOL/CSA%20Solutioning/_workitems/edit/43311

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…DO #43311)

Adds a Data Collection Rule that captures Windows audit success (EventID 4624) and
audit failure (EventID 4625) Security events from the jumpbox VM and routes them
to the Log Analytics workspace via the Microsoft-SecurityEvent stream. The DCR is
associated with the VM through the Azure Monitor Agent extension
(extensionMonitoringAgentConfig.dataCollectionRuleAssociations). The OMSGallery
Security solution is installed on the workspace so the SecurityEvent table is
populated for the routed stream.

Pattern mirrors microsoft/Modernize-your-code-solution-accelerator#435 but the
audit success and audit failure events are covered by a single xPath
(Security!*[System[(EventID=4624 or EventID=4625)]]) routed via the
Microsoft-SecurityEvent stream rather than Microsoft-WindowsEvent.

All new resources are gated on enablePrivateNetworking && enableMonitoring so
non-WAF / non-monitoring deployments are unaffected.

Files touched:
- infra/main.bicep         (jumpboxVM AMA extension; new securitySolution +
                            windowsVmDataCollectionRules)
- infra/main_custom.bicep  (same additions)

Addresses SFI item: "data collection rule ['audit success','audit failure']
logs should be enabled".

Work item: AB#43311
ADO: https://dev.azure.com/CSACTOSOL/CSA%20Solutioning/_workitems/edit/43311

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Regenerated infra/main.json via 'az bicep build infra/main.bicep' to pick up the
four SFI changes on this branch:
  * Container Apps Environment peerTrafficEncryption
  * Storage account requireInfrastructureEncryption
  * SystemAssigned identity on cosmos / ACR / jumpbox VM
  * Windows Security audit DCR + OMSGallery/Security solution

The large diff is dominated by the inlined AVM data-collection-rule:0.11.0 module
definition pulled into main.json by the new windowsVmDataCollectionRules module.
No main_custom.json exists in this repo (main_custom.bicep is consumed by tooling
that runs bicep on demand).

Work item: AB#43311
ADO: https://dev.azure.com/CSACTOSOL/CSA%20Solutioning/_workitems/edit/43311

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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

Note

Copilot was unable to run its full agentic suite in this review.

This PR addresses SFI (Secure Future Initiative) compliance items flagged by the EXP team (ADO #43311). It is an infra-only change, applied symmetrically to infra/main.bicep and infra/main_custom.bicep, with infra/main.json regenerated.

Changes:

  • Enable peerTrafficEncryption on the Container Apps Environment and requireInfrastructureEncryption on storage accounts.
  • Add system-assigned managed identities to Cosmos DB, Azure Container Registry, and the jumpbox VM.
  • Add an OMS Security solution and a Windows Security audit (EventID 4624/4625) Data Collection Rule, associated with the jumpbox VM via the Azure Monitor Agent.

Reviewed changes

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

File Description
infra/modules/storageAccount.bicep Explicitly enable infrastructure encryption and set keyType: 'Service'.
infra/main.bicep Add identities, DCR + Security solution, peer traffic encryption, and storage infra encryption.
infra/main_custom.bicep Mirror of main.bicep changes for the custom-deployment path.
infra/main.json Regenerated ARM template reflecting Bicep changes.

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

Comment thread infra/main.bicep Outdated
Comment thread infra/main.bicep Outdated
Comment thread infra/main.bicep Outdated
Comment thread infra/main.bicep Outdated
Comment thread infra/main.bicep
Comment thread infra/main.bicep
The Microsoft.OperationsManagement/solutions 'Security' (OMSGallery/Security)
resource is a legacy artifact from the Log Analytics Agent (MMA) era. With the
modern Azure Monitor Agent + Data Collection Rule pipeline used here, the
SecurityEvent table is auto-provisioned by Azure Monitor on first ingestion
of the Microsoft-SecurityEvent stream. The OMSGallery solution is not
required and adds a marketplace plan resource for no functional benefit.

- Remove the securitySolution resource from infra/main.bicep and
  infra/main_custom.bicep.
- Drop the now-unnecessary dependsOn: [securitySolution] from the
  windowsVmDataCollectionRules module.
- Regenerate infra/main.json from main.bicep.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@Shreyas-Microsoft Shreyas-Microsoft changed the title fix(infra): address SFI security compliance issues (ADO #43311) fix(infra): address SFI security compliance issues May 21, 2026
Comment thread infra/main.bicep Outdated
Address review comments from @Harsh-Microsoft on PR #244:

- xPathQueries: broaden the Windows Security filter to use the audit
  Keywords bitmask (0x30000000000000 = AuditSuccess|AuditFailure) and
  exclude the very high-volume EventID 4624 (successful logon). The new
  filter captures all audit success and audit failure events (which is
  what the SFI item actually asks for), not just 4624/4625, while
  keeping ingestion cost sane.
- dataSources: add a small Microsoft-Perf set (CPU, memory, disk, network)
  to the same DCR so the jumpbox VM also produces a baseline VM health
  signal alongside the security audit stream.
- dcrLogAnalyticsDestinationName: extract the destination name to a
  variable (\la-<lawName>-destination\) and reference it from the
  destinations.logAnalytics entry and both dataFlows (SecurityEvent and
  Perf) so the name is defined once.

Applied to both infra/main.bicep and infra/main_custom.bicep, and
regenerated infra/main.json from main.bicep.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings May 22, 2026 05:33
Comment thread infra/main.bicep Outdated
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 4 changed files in this pull request and generated 2 comments.

Comment thread infra/main.bicep
Comment thread infra/main_custom.bicep
…#43311)

Align the jumpbox VM's Microsoft-Perf counterSpecifiers list with the
pattern established in microsoft/Multi-Agent-Custom-Automation-Engine-
Solution-Accelerator PR #989, which uses a comprehensive 46-counter set
named 'perfCounterDataSource60'. Previously this PR shipped a minimal
7-counter set; the expanded list adds detailed Processor (privileged/user
time, frequency), System (uptime, context switches, queue length),
Process (handle/thread counts, working set), Memory (committed bytes,
cache, paged/nonpaged pool, page faults), LogicalDisk (read/write
time/bytes/transfers, queue lengths, free space) and Network Interface
(sent/received bytes and packets, errors) counters.

Same set applied to infra/main.bicep and infra/main_custom.bicep, with
infra/main.json regenerated from main.bicep.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Address PR #244 review feedback:

- Gate jumpbox VM's extensionMonitoringAgentConfig on the same
  (enablePrivateNetworking && enableMonitoring) expression as the
  windowsVmDataCollectionRules module so the dereference of
  windowsVmDataCollectionRules!.outputs.resourceId stays safe even if
  the outer jumpbox VM gate ever changes (previously gated only on
  enableMonitoring, which works today only because the outer module is
  gated on enablePrivateNetworking).
- Reword the comment above the extensionMonitoringAgentConfig block to
  drop the now-misleading '(4624) / (4625)' wording (4624 is excluded
  by the audit Keywords bitmask filter to control ingestion volume).
- Remove all '(ADO #43311)' references from SFI comments in
  infra/main.bicep, infra/main_custom.bicep, and
  infra/modules/storageAccount.bicep.

Applied to both infra/main.bicep and infra/main_custom.bicep, with
infra/main.json regenerated from main.bicep.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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 4 changed files in this pull request and generated 1 comment.

Comment thread infra/main.bicep
@Prajwal-Microsoft Prajwal-Microsoft merged commit cbc17d0 into dev May 22, 2026
10 checks passed
@github-actions
Copy link
Copy Markdown

🎉 This PR is included in version 2.1.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants