From a7b24bbb8bd4918e63782f085b18fafb7cf7ab28 Mon Sep 17 00:00:00 2001 From: Jeffrey Bulanadi <41933086+jeffreybulanadi@users.noreply.github.com> Date: Sat, 2 May 2026 06:37:16 +0800 Subject: [PATCH 1/2] Preserve overwriteSettings across template source merges in ReadSettings When a user repo settings file (AL-Go-Settings.json) uses overwriteSettings to clear or replace an array property such as appDependencyProbingPaths, a subsequent template-managed source file (AL-Go-TemplateProjectSettings.doNotEdit.json) was able to re-merge its own values into that property because the MergeCustomObjectIntoOrderedDictionary function had no awareness of which settings had already been explicitly overwritten by a user source. Changes: - MergeCustomObjectIntoOrderedDictionary gains two optional parameters: overwrittenSettings ([string[]]) - names of settings already overwritten by a user-controlled source, and isTemplateSource ([bool]) - true when the source file is a template-managed *.doNotEdit.json file. When isTemplateSource is true and a property name appears in overwrittenSettings, the merge of that property is skipped entirely. - ReadSettings now tracks overwrittenSettings as it iterates the ordered settings sources. After each non-template source is merged, any property names declared in its overwriteSettings list are accumulated. Template sources that follow receive the accumulated list so they cannot re-introduce overwritten values. - Two Pester tests added to ReadSettings.Test.ps1 covering: 1. overwriteSettings in a user source prevents template project settings from re-merging that property. 2. overwriteSettings in a user source still allows later user project settings to contribute additional values to that property. --- Actions/.Modules/ReadSettings.psm1 | 36 +++++++++++-- Tests/ReadSettings.Test.ps1 | 86 ++++++++++++++++++++++++++++++ 2 files changed, 119 insertions(+), 3 deletions(-) diff --git a/Actions/.Modules/ReadSettings.psm1 b/Actions/.Modules/ReadSettings.psm1 index ea3bbe350a..7a0afe41f8 100644 --- a/Actions/.Modules/ReadSettings.psm1 +++ b/Actions/.Modules/ReadSettings.psm1 @@ -16,7 +16,11 @@ $CustomTemplateProjectSettingsFile = Join-Path '.github' $CustomTemplateProjectS function MergeCustomObjectIntoOrderedDictionary { Param( [System.Collections.Specialized.OrderedDictionary] $dst, - [PSCustomObject] $src + [PSCustomObject] $src, + # Settings already explicitly overwritten by a prior user source. Template sources must not re-apply these. + [string[]] $overwrittenSettings = @(), + # True when the source file is a template-managed file (*.doNotEdit.json). Used together with $overwrittenSettings. + [bool] $isTemplateSource = $false ) # If the src object contains property 'overwriteSettings' (list of settings), remove these settings from the dst object, so that they can be re-added with the new value later on @@ -42,6 +46,11 @@ function MergeCustomObjectIntoOrderedDictionary { return } + # Template sources must not re-introduce a property that was explicitly overwritten by a user source + if ($isTemplateSource -and $overwrittenSettings -contains $prop) { + return + } + $srcProp = $src."$prop" $srcPropType = $srcProp.GetType().Name if (-not $dst.Contains($prop)) { @@ -67,6 +76,11 @@ function MergeCustomObjectIntoOrderedDictionary { @($dst.Keys) | ForEach-Object { $prop = $_ if ($src.PSObject.Properties.Name -eq $prop) { + # Template sources must not overwrite a setting that was explicitly overwritten by a user source + if ($isTemplateSource -and $overwrittenSettings -contains $prop) { + OutputDebug "Skipping template merge of explicitly overwritten setting $prop" + return + } $dstProp = $dst."$prop" $srcProp = $src."$prop" $dstPropType = $dstProp.GetType().Name @@ -479,11 +493,27 @@ function ReadSettings { } } + # Tracks settings names that were explicitly overwritten by a user-controlled source. + # Template sources (*.doNotEdit.json) that appear later in the merge order must not re-apply these. + $overwrittenSettings = @() + foreach($settingsObject in $settingsObjects) { $settingsJson = $settingsObject.Settings if ($settingsJson) { + $isTemplateSource = $settingsObject.Source -like "*.doNotEdit.json" OutputDebug "Applying settings from $($settingsObject.Source) ($($settingsObject.Type))" - MergeCustomObjectIntoOrderedDictionary -dst $settings -src $settingsJson + MergeCustomObjectIntoOrderedDictionary -dst $settings -src $settingsJson -overwrittenSettings $overwrittenSettings -isTemplateSource $isTemplateSource + + # After a non-template source is merged, record which settings it declared as overwritten. + # Subsequent template sources must not re-apply those settings. + if (-not $isTemplateSource -and $settingsJson.PSObject.Properties.Name -contains "overwriteSettings") { + $settingsJson.overwriteSettings | ForEach-Object { + if ($_ -notin $overwrittenSettings) { + $overwrittenSettings += $_ + } + } + } + if ($settingsJson.PSObject.Properties.Name -eq "ConditionalSettings") { foreach($conditionalSetting in $settingsJson.ConditionalSettings) { if ("$conditionalSetting" -ne "") { @@ -505,7 +535,7 @@ function ReadSettings { } if ($conditionMet) { OutputDebug "Applying conditional settings for $($conditions -join ", ")" - MergeCustomObjectIntoOrderedDictionary -dst $settings -src $conditionalSetting.settings + MergeCustomObjectIntoOrderedDictionary -dst $settings -src $conditionalSetting.settings -overwrittenSettings $overwrittenSettings -isTemplateSource $isTemplateSource } } } diff --git a/Tests/ReadSettings.Test.ps1 b/Tests/ReadSettings.Test.ps1 index 24fdb69c9a..de4d426453 100644 --- a/Tests/ReadSettings.Test.ps1 +++ b/Tests/ReadSettings.Test.ps1 @@ -460,6 +460,92 @@ InModuleScope ReadSettings { # Allows testing of private functions $dst.PSObject.Properties.Name | Should -Not -Contain 'overwriteSettings' } + It 'overwriteSettings in a user source prevents subsequent template sources from re-merging that setting' { + Mock Write-Host { } + Mock Out-Host { } + + Push-Location + $tempName = Join-Path ([System.IO.Path]::GetTempPath()) ([Guid]::NewGuid().ToString()) + $githubFolder = Join-Path $tempName ".github" + $projectALGoFolder = Join-Path $tempName "Project/$ALGoFolderName" + + New-Item $githubFolder -ItemType Directory | Out-Null + New-Item $projectALGoFolder -ItemType Directory | Out-Null + + try { + # Template repo settings add probing paths + @{ "appDependencyProbingPaths" = @( @{ "repo" = "template-repo"; "version" = "latest" } ) } | ConvertTo-Json -Depth 99 | + Set-Content -Path (Join-Path $githubFolder $CustomTemplateRepoSettingsFileName) -Encoding utf8 -Force + + # User repo settings explicitly overwrite appDependencyProbingPaths to empty + @{ "overwriteSettings" = @("appDependencyProbingPaths"); "appDependencyProbingPaths" = @() } | ConvertTo-Json -Depth 99 | + Set-Content -Path (Join-Path $githubFolder "AL-Go-Settings.json") -Encoding utf8 -Force + + # Template project settings also have probing paths (this is the source of the bug) + @{ "appDependencyProbingPaths" = @( @{ "repo" = "template-project-repo"; "version" = "latest" } ) } | ConvertTo-Json -Depth 99 | + Set-Content -Path (Join-Path $githubFolder $CustomTemplateProjectSettingsFileName) -Encoding utf8 -Force + + $ENV:ALGoOrgSettings = '' + $ENV:ALGoRepoSettings = '' + + $settings = ReadSettings -baseFolder $tempName -project 'Project' -repoName 'repo' -workflowName '' -branchName '' -userName '' + + # appDependencyProbingPaths must remain empty: the user's overwriteSettings must win over both template sources + $settings.appDependencyProbingPaths | Should -BeNullOrEmpty + } + finally { + Pop-Location + Remove-Item $tempName -Recurse -Force + } + } + + It 'overwriteSettings in a user source still allows subsequent user sources to contribute to that setting' { + Mock Write-Host { } + Mock Out-Host { } + + Push-Location + $tempName = Join-Path ([System.IO.Path]::GetTempPath()) ([Guid]::NewGuid().ToString()) + $githubFolder = Join-Path $tempName ".github" + $projectALGoFolder = Join-Path $tempName "Project/$ALGoFolderName" + + New-Item $githubFolder -ItemType Directory | Out-Null + New-Item $projectALGoFolder -ItemType Directory | Out-Null + + try { + # Template repo settings add probing paths + @{ "appDependencyProbingPaths" = @( @{ "repo" = "template-repo"; "version" = "latest" } ) } | ConvertTo-Json -Depth 99 | + Set-Content -Path (Join-Path $githubFolder $CustomTemplateRepoSettingsFileName) -Encoding utf8 -Force + + # User repo settings explicitly overwrite appDependencyProbingPaths to a specific set + @{ "overwriteSettings" = @("appDependencyProbingPaths"); "appDependencyProbingPaths" = @( @{ "repo" = "user-repo"; "version" = "1.0" } ) } | ConvertTo-Json -Depth 99 | + Set-Content -Path (Join-Path $githubFolder "AL-Go-Settings.json") -Encoding utf8 -Force + + # Template project settings also have probing paths (must be ignored) + @{ "appDependencyProbingPaths" = @( @{ "repo" = "template-project-repo"; "version" = "latest" } ) } | ConvertTo-Json -Depth 99 | + Set-Content -Path (Join-Path $githubFolder $CustomTemplateProjectSettingsFileName) -Encoding utf8 -Force + + # User project settings add one more probing path (must be merged on top of the user repo overwrite) + @{ "appDependencyProbingPaths" = @( @{ "repo" = "project-repo"; "version" = "2.0" } ) } | ConvertTo-Json -Depth 99 | + Set-Content -Path (Join-Path $projectALGoFolder "settings.json") -Encoding utf8 -Force + + $ENV:ALGoOrgSettings = '' + $ENV:ALGoRepoSettings = '' + + $settings = ReadSettings -baseFolder $tempName -project 'Project' -repoName 'repo' -workflowName '' -branchName '' -userName '' + + # Only user-repo and project-repo entries must be present; template entries must be absent + $settings.appDependencyProbingPaths.Count | Should -Be 2 + $settings.appDependencyProbingPaths.repo | Should -Contain 'user-repo' + $settings.appDependencyProbingPaths.repo | Should -Contain 'project-repo' + $settings.appDependencyProbingPaths.repo | Should -Not -Contain 'template-repo' + $settings.appDependencyProbingPaths.repo | Should -Not -Contain 'template-project-repo' + } + finally { + Pop-Location + Remove-Item $tempName -Recurse -Force + } + } + It 'Multiple conditionalSettings with same array setting are merged (all entries kept)' { Mock Write-Host { } Mock Out-Host { } From 02292642d48b63af302a1b5481a8aade45696508 Mon Sep 17 00:00:00 2001 From: Jeffrey Bulanadi <41933086+jeffreybulanadi@users.noreply.github.com> Date: Tue, 5 May 2026 04:32:19 +0800 Subject: [PATCH 2/2] ReadSettings: guard overwrittenSettings accumulation with property presence check Previously, any setting listed in overwriteSettings was added to the overwrittenSettings tracker regardless of whether the source object actually provided a value for that setting. This diverged from the behavior in MergeCustomObjectIntoOrderedDictionary, which only clears a destination setting when the source contains both the overwriteSettings declaration and the property itself. The consequence: a user source that lists a setting in overwriteSettings but omits the property value would silently block all subsequent template sources from contributing that setting, effectively dropping intended template defaults with no value provided by the user. The accumulation now applies the same presence check: a setting is recorded as overwritten only when the source object contains the property in addition to declaring it in overwriteSettings. A test is added covering the case where overwriteSettings is declared without the corresponding property, verifying that the template source value reaches the final settings. --- Actions/.Modules/ReadSettings.psm1 | 5 +++- Tests/ReadSettings.Test.ps1 | 39 +++++++++++++++++++++++++++++- 2 files changed, 42 insertions(+), 2 deletions(-) diff --git a/Actions/.Modules/ReadSettings.psm1 b/Actions/.Modules/ReadSettings.psm1 index 7a0afe41f8..354f9af9b7 100644 --- a/Actions/.Modules/ReadSettings.psm1 +++ b/Actions/.Modules/ReadSettings.psm1 @@ -506,9 +506,12 @@ function ReadSettings { # After a non-template source is merged, record which settings it declared as overwritten. # Subsequent template sources must not re-apply those settings. + # A setting is only considered overwritten when the source both declares it in overwriteSettings + # and also provides the property value. This mirrors the guard in MergeCustomObjectIntoOrderedDictionary + # that only removes a destination setting when the source contains the property as well. if (-not $isTemplateSource -and $settingsJson.PSObject.Properties.Name -contains "overwriteSettings") { $settingsJson.overwriteSettings | ForEach-Object { - if ($_ -notin $overwrittenSettings) { + if ($_ -notin $overwrittenSettings -and $settingsJson.PSObject.Properties.Name -contains $_) { $overwrittenSettings += $_ } } diff --git a/Tests/ReadSettings.Test.ps1 b/Tests/ReadSettings.Test.ps1 index de4d426453..4294c29e73 100644 --- a/Tests/ReadSettings.Test.ps1 +++ b/Tests/ReadSettings.Test.ps1 @@ -546,7 +546,44 @@ InModuleScope ReadSettings { # Allows testing of private functions } } - It 'Multiple conditionalSettings with same array setting are merged (all entries kept)' { + It 'overwriteSettings in a user source without the property value does not block template sources from merging that setting' { + Mock Write-Host { } + Mock Out-Host { } + + Push-Location + $tempName = Join-Path ([System.IO.Path]::GetTempPath()) ([Guid]::NewGuid().ToString()) + $githubFolder = Join-Path $tempName ".github" + $projectALGoFolder = Join-Path $tempName "Project/$ALGoFolderName" + + New-Item $githubFolder -ItemType Directory | Out-Null + New-Item $projectALGoFolder -ItemType Directory | Out-Null + + try { + # Template repo settings add probing paths + @{ "appDependencyProbingPaths" = @( @{ "repo" = "template-repo"; "version" = "latest" } ) } | ConvertTo-Json -Depth 99 | + Set-Content -Path (Join-Path $githubFolder $CustomTemplateRepoSettingsFileName) -Encoding utf8 -Force + + # User repo settings declare overwriteSettings for appDependencyProbingPaths but do NOT provide the property value. + # This must not block template sources from contributing appDependencyProbingPaths. + @{ "overwriteSettings" = @("appDependencyProbingPaths") } | ConvertTo-Json -Depth 99 | + Set-Content -Path (Join-Path $githubFolder "AL-Go-Settings.json") -Encoding utf8 -Force + + $ENV:ALGoOrgSettings = '' + $ENV:ALGoRepoSettings = '' + + $settings = ReadSettings -baseFolder $tempName -project 'Project' -repoName 'repo' -workflowName '' -branchName '' -userName '' + + # appDependencyProbingPaths must contain the template entry because the user source did not provide a value + $settings.appDependencyProbingPaths | Should -Not -BeNullOrEmpty + $settings.appDependencyProbingPaths.repo | Should -Contain 'template-repo' + } + finally { + Pop-Location + Remove-Item $tempName -Recurse -Force + } + } + + Mock Write-Host { } Mock Out-Host { }