Fix template resolution type coercion for pure placeholders#167
Fix template resolution type coercion for pure placeholders#167
Conversation
Co-authored-by: blindzero <13959569+blindzero@users.noreply.github.com>
Co-authored-by: blindzero <13959569+blindzero@users.noreply.github.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2308449431
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Pull request overview
Fixes IdLE workflow template resolution so that pure placeholders (a value that is exactly {{...}} with no surrounding text) preserve the resolved value’s original type instead of coercing to string, preventing typed workflow parameters (notably booleans) from being silently flipped during casting.
Changes:
- Add pure-placeholder detection in
Resolve-IdleTemplateStringand return typed resolved values for that case; keep string interpolation behavior for mixed templates. - Refactor shared path validation/root allowlist into reusable scriptblocks.
- Add fixtures + Pester tests covering type preservation for bool/int/datetime/guid and confirming mixed templates still interpolate to strings.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/IdLE.Core/Private/Resolve-IdleTemplateString.ps1 |
Adds pure-placeholder typed return path while keeping mixed-template string interpolation and existing security/root validation. |
tests/Core/Resolve-IdleWorkflowTemplates.Tests.ps1 |
Adds regression tests ensuring scalar types are preserved for pure placeholders and strings remain strings for mixed templates. |
tests/fixtures/workflows/template-tests/template-pure-boolean-false.psd1 |
Fixture workflow for pure-placeholder boolean $false. |
tests/fixtures/workflows/template-tests/template-pure-boolean-true.psd1 |
Fixture workflow for pure-placeholder boolean $true. |
tests/fixtures/workflows/template-tests/template-pure-integer.psd1 |
Fixture workflow for pure-placeholder integer. |
tests/fixtures/workflows/template-tests/template-pure-datetime.psd1 |
Fixture workflow for pure-placeholder datetime. |
tests/fixtures/workflows/template-tests/template-pure-guid.psd1 |
Fixture workflow for pure-placeholder guid. |
tests/fixtures/workflows/template-tests/template-mixed-boolean.psd1 |
Fixture workflow for mixed-template string interpolation with a boolean. |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: blindzero <13959569+blindzero@users.noreply.github.com>
Co-authored-by: blindzero <13959569+blindzero@users.noreply.github.com>
Co-authored-by: blindzero <13959569+blindzero@users.noreply.github.com>
|
@copilot apply changes based on the comments in this thread |
Co-authored-by: blindzero <13959569+blindzero@users.noreply.github.com>
Fix Template Resolution Type Coercion Issue ✅
Successfully fixed template resolution to preserve types for pure placeholders instead of unconditionally coercing to strings.
Summary of Changes
Problem: Template resolution was converting all resolved values to strings, breaking boolean and other typed parameters. For example,
$false→"False"→[bool]"False"=$true.Solution: Modified
Resolve-IdleTemplateString.ps1to detect pure placeholders (single placeholder with no surrounding text) and return the resolved value with its original type preserved.Implementation Details
Resolve-IdleTemplateString.ps1Resolve-IdleWorkflowTemplates.Tests.ps1Resolve-IdleTemplateString.ps1to detect pure placeholdersChanges Made
Modified:
src/IdLE.Core/Private/Resolve-IdleTemplateString.ps1^\s*\{\{([^}]+)\}\}\s*$$resolvePathhelper.IsValueTypepropertyAdded:
tests/fixtures/workflows/template-tests/template-pure-boolean-false.psd1tests/fixtures/workflows/template-tests/template-pure-boolean-true.psd1tests/fixtures/workflows/template-tests/template-pure-integer.psd1tests/fixtures/workflows/template-tests/template-pure-datetime.psd1tests/fixtures/workflows/template-tests/template-pure-guid.psd1tests/fixtures/workflows/template-tests/template-mixed-boolean.psd1Updated:
tests/Core/Resolve-IdleWorkflowTemplates.Tests.ps1Test Results
✅ All 198 Core tests pass (0 failures, 2 skipped)
✅ All 32 template resolution tests pass (including 6 new type preservation tests)
✅ ScriptAnalyzer passes with no findings
✅ CodeQL security check passes
✅ Manual verification confirms boolean false stays false
✅ Security checks apply to both pure and mixed templates (verified manually)
Code Quality & Security Improvements
.IsValueTypeType Validation Logic
Allowed types (data-only scalars):
.IsValueType:Explicitly rejected (security - applies to both pure and mixed templates):
Backwards Compatibility
This is a bug fix with no breaking changes:
Original prompt
This section details on the original issue you should resolve
<issue_title>Template resolution coerces scalars to string (breaks booleans like Enabled=$false)</issue_title>
<issue_description>## Description
IdLE template resolution currently coerces resolved placeholder values to string unconditionally.
This breaks scenarios where a step expects a typed scalar (e.g.
bool) and the workflow uses a template placeholder like:Enabled = '{{Request.DesiredState.Enabled}}'Even if
Request.DesiredState.Enabledis a real boolean$false, the template resolver returns"False"(a non-empty string). In PowerShell, casting a non-empty string to[bool]yields$true, so the user ends up enabled unexpectedly.This is observed with
IdLE.Step.CreateIdentity(AD provider), whereEnabledis derived fromAttributes['Enabled']via[bool]conversion.This is a fail-fast correctness bug: it silently flips user intent for boolean parameters.
Step-0 Analysis (Safe)
What happens today
Request contains a real boolean:
DesiredState.Enabled = $falseWorkflow contains a template placeholder:
Enabled = '{{Request.DesiredState.Enabled}}'The template resolver resolves placeholders and coerces to string (effectively
[string]$resolvedValue).The AD provider adapter reads the resolved value:
$enabled = [bool]$Attributes['Enabled']In PowerShell:
[bool]"False"is$true(because it's a non-empty string).Why this is not a user error
Steps to Reproduce
Create a request:
Use a workflow step that templates
Enabled:Execute the plan (any host runner).
Expected Behavior
Enabledshould remain a booleanFalseafter template resolution.$enabled -eq $false).Actual Behavior
"False"(string).$true.Enabledbehaves inverted).Scope / Impact
Proposed Fix (Agent-Safe)
Fix rule
When a workflow value is exactly one template placeholder (no prefix/suffix text), the template engine must return the resolved value as-is (typed
object) instead of coercing to string.Examples:
Input:
'{{Request.DesiredState.Enabled}}'bool(same asRequest.DesiredState.Enabled)Input:
'Enabled={{Request.DesiredState.Enabled}}'(mixed string)string(string interpolation remains valid)Implementation outline
In the template resolution function (currently
Resolve-IdleTemplateStringor wherever the placeholder substitution is performed):^\s*\{\{.+?\}\}\s*$object) without converting to string.Ensure this behavior is applied consistently wherever workflow templates are resolved (arrays/hashtables/nested structures).
Add unit tests for:
$falsestays$false(typed)$truestays$trueAdd a regression test for the AD CreateIdentity scenario:
$falsefor Enabled when templated.Backwards compatibility
This is a bug fix. Typed preservation only happens for pure placeholders and should not break existing string interpolation patterns.
Additional Notes / References
[bool]$Attributes['Enabled'], which is correct given a b...💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.