Skip to content

Fix AD provider LDAP escaping scope issue in ScriptMethod execution#158

Merged
blindzero merged 4 commits intomainfrom
copilot/fix-ad-provider-runtime-error
Feb 8, 2026
Merged

Fix AD provider LDAP escaping scope issue in ScriptMethod execution#158
blindzero merged 4 commits intomainfrom
copilot/fix-ad-provider-runtime-error

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Feb 8, 2026

  • Review and understand the scope issue with Protect-LdapFilterValue in New-IdleADAdapter.ps1
  • Implement fix: Add ProtectLdapFilterValue as a ScriptMethod on the adapter object
  • Update all adapter ScriptMethods to call $this.ProtectLdapFilterValue() instead of Protect-LdapFilterValue
  • Remove the nested function Protect-LdapFilterValue to avoid confusion
  • Add unit tests for the LDAP escaping functionality
  • Run AD provider tests to ensure the fix works (36 tests passed)
  • Verify no regressions with PSScriptAnalyzer (no issues found)
  • Address code review feedback:
    • Added single quote escaping to GetUserBySam (consistent with GetUserByUpn and ListUsers)
    • Updated tests to use real adapter implementation instead of re-implementing logic
    • Fixed misleading comment in test file
Original prompt

This section details on the original issue you should resolve

<issue_title>IdLE.Provider.AD fails at runtime because Protect-LdapFilterValue is not in scope for adapter ScriptMethods</issue_title>
<issue_description>## Description

The Active Directory identity provider fails at runtime when resolving an identity by sAMAccountName. The error indicates that the helper function Protect-LdapFilterValue is not recognized, causing identity resolution to fail and consequently breaking steps like IdLE.Step.DisableIdentity.

This is a regression/defect in the AD adapter implementation: Protect-LdapFilterValue is defined as a nested function within New-IdleADAdapter, but it is referenced from ScriptMethod members (e.g., GetUserBySam). When these ScriptMethods are invoked later, the nested function is out of scope, so PowerShell cannot resolve it.

Impact: AD-based workflows (e.g., Leaver DisableIdentity) cannot execute reliably.

Steps to Reproduce

  1. Install/import IdLE modules including the AD provider.
  2. Configure an AD identity provider and run a workflow containing IdLE.Step.DisableIdentity with an IdentityKey that resolves via sAMAccountName (e.g., foo.bar).
  3. Build and invoke the plan.

Example minimal workflow step:

@{
  Name           = 'Leave Account Test'
  LifecycleEvent = 'Leaver'
  Steps          = @(
    @{
      Name = 'Disable AD account'
      Type = 'IdLE.Step.DisableIdentity'
      With = @{
        AuthSessionName = 'AD'
        IdentityKey     = 'foo.bar'
      }
    }
  )
}

Example plan creation (provider setup omitted here, see Additional Context):

$leaverReq  = New-IdleLifecycleRequest -LifecycleEvent 'Leaver'
$leaverPlan = New-IdlePlan -WorkflowPath .\leaver.psd1 -Request $leaverReq -Providers $providers
Invoke-IdlePlan -Plan $leaverPlan

Expected Behavior

  • IdLE.Step.DisableIdentity resolves the identity using the AD provider and disables the AD account.
  • No missing-command errors occur.
  • LDAP filter values are properly escaped prior to querying AD.

Actual Behavior

The step fails with an exception chain that ends in:

The term 'Protect-LdapFilterValue' is not recognized as a name of a cmdlet, function, script file, or executable program.

Example error (truncated):

Exception calling "DisableIdentity" ... 
Exception calling "ResolveIdentity" ...
Exception calling "GetUserBySam" ...
The term 'Protect-LdapFilterValue' is not recognized ...

Environment

  • PowerShell version: (fill in)
  • OS: (fill in)
  • IdLE version / commit: (fill in; e.g., 0.9.2)
  • Execution context (CLI / Service / CI): (fill in)
  • Provider: IdLE.Provider.AD

Root Cause Analysis (Proposed)

  • Protect-LdapFilterValue is implemented as a nested function inside New-IdleADAdapter.ps1.
  • AD adapter operations are exposed as ScriptMethods (e.g., GetUserBySam) on an object created by New-IdleADAdapter.
  • When a ScriptMethod runs, it does not retain the lexical scope of New-IdleADAdapter, therefore the nested function is not visible.
  • Result: calling Protect-LdapFilterValue inside the ScriptMethod throws a command not recognized error.

Proposed Fix

Make the LDAP escaping helper available in the execution scope of the adapter ScriptMethods in a testable, non-global way.

Preferred approach (Option A):

  1. Add a ScriptMethod on the adapter object, e.g. ProtectLdapFilterValue, implemented directly on the adapter.
  2. Update all adapter ScriptMethods that need escaping (e.g., GetUserBySam, GetUserByUpn, any search methods) to call:
    • $this.ProtectLdapFilterValue($value)
  3. Remove the nested function version to avoid future scope confusion.

Alternative approach (Option B):

  • Move Protect-LdapFilterValue to a private module function file (e.g., Private/Protect-LdapFilterValue.ps1) within the AD provider module and ensure it is dot-sourced/exported appropriately for internal use (no global pollution), then reference it from ScriptMethods.

Acceptance Criteria

  • Running a workflow with IdLE.Step.DisableIdentity against AD no longer fails with missing Protect-LdapFilterValue.
  • AD identity resolution by sAMAccountName works end-to-end.
  • LDAP filter escaping is still applied and covered by tests.
  • No new global functions are introduced (no Global scope pollution).
  • Docs/examples that demonstrate AD workflows still work without additional steps.

Test Plan

Unit Tests (Pester)

  • Add/extend unit tests for the AD adapter to validate that calling GetUserBySam does not throw due to missing helper function.
  • Add tests for ProtectLdapFilterValue behavior (escape sequences for \, *, (, ), null byte).
  • Add a unit/integration-style test using the mock provider (or a minimal adapter stub) to confirm ResolveIdentity can call GetUserBySam successfully.

Regression Coverage

  • A test that simulates a full DisableIdentity...

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Co-authored-by: blindzero <13959569+blindzero@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix runtime error in IdLE.Provider.AD for Protect-LdapFilterValue Fix AD provider LDAP escaping scope issue in ScriptMethod execution Feb 8, 2026
Copilot AI requested a review from blindzero February 8, 2026 15:49
@blindzero blindzero marked this pull request as ready for review February 8, 2026 15:53
Copilot AI review requested due to automatic review settings February 8, 2026 15:53
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

Fixes a runtime failure in the AD provider by making the LDAP filter escaping helper available to adapter ScriptMethods (which don’t capture the parent function scope), and adds regression tests around escaping behavior.

Changes:

  • Converts LDAP escaping helper into an adapter ScriptMethod (ProtectLdapFilterValue) so other adapter ScriptMethods can call it via $this.
  • Updates AD adapter methods (GetUserByUpn, GetUserBySam, ListUsers) to call $this.ProtectLdapFilterValue(...).
  • Adds Pester coverage intended to validate escaping behavior and the ScriptMethod scoping regression.

Reviewed changes

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

File Description
src/IdLE.Provider.AD/Private/New-IdleADAdapter.ps1 Moves LDAP escaping into an adapter ScriptMethod and updates callers to use $this.
tests/Providers/ADIdentityProvider.Tests.ps1 Adds tests for escaping and for calling the helper from within another ScriptMethod.
Comments suppressed due to low confidence (1)

src/IdLE.Provider.AD/Private/New-IdleADAdapter.ps1:78

  • Same issue as above: $escapedSam is injected into a single-quoted Get-ADUser -Filter string without escaping '. This can cause parse errors or filter injection for sAMAccountName values containing an apostrophe.
        $escapedSam = $this.ProtectLdapFilterValue($SamAccountName)

        $params = @{
            Filter     = "sAMAccountName -eq '$escapedSam'"
            Properties = @('Enabled', 'DistinguishedName', 'ObjectGuid', 'UserPrincipalName', 'sAMAccountName')

Comment thread src/IdLE.Provider.AD/Private/New-IdleADAdapter.ps1
Comment thread src/IdLE.Provider.AD/Private/New-IdleADAdapter.ps1 Outdated
Comment thread tests/Providers/ADIdentityProvider.Tests.ps1 Outdated
Comment thread tests/Providers/ADIdentityProvider.Tests.ps1 Outdated
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copy link
Copy Markdown
Owner

@blindzero blindzero left a comment

Choose a reason for hiding this comment

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

@copilot check open suggestion comments from reviews

…am and test real implementation

Co-authored-by: blindzero <13959569+blindzero@users.noreply.github.com>
Copy link
Copy Markdown
Owner

@blindzero blindzero left a comment

Choose a reason for hiding this comment

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

@copilot check open suggestion comments from reviews

@blindzero blindzero merged commit 622ef01 into main Feb 8, 2026
5 checks passed
@blindzero blindzero deleted the copilot/fix-ad-provider-runtime-error branch February 9, 2026 22:12
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.

IdLE.Provider.AD fails at runtime because Protect-LdapFilterValue is not in scope for adapter ScriptMethods

3 participants