Add bulk agent registration scripts - #492
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: c1ad322d-d2ae-483e-a7a8-6fdc792ac76f
There was a problem hiding this comment.
🟡 Changes recommended
Remove-A365AgentRegistration.ps1 contains misleading user-facing error messages and a step-numbering output bug that should be corrected before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR adds PowerShell entry-point scripts under scripts/bulk-agent-registration/ to support bulk update and removal operations for Microsoft Agent 365 resources, delegating most Graph work to the existing New-* step scripts where applicable.
Changes:
- Added
Update-*wrapper scripts that forward only explicitly bound parameters to the underlyingNew-*scripts in “update mode”. - Added
Remove-*scripts for deleting blueprints, agent registrations, agent users, and agent identities (with logging, planning output, and safety checks for typed resources). - Implemented shared logging/redaction patterns across the new removal scripts.
File summaries
| File | Description |
|---|---|
| scripts/bulk-agent-registration/Update-A365Blueprint.ps1 | Update wrapper for blueprints that forwards only bound attributes to the step script. |
| scripts/bulk-agent-registration/Update-A365AgentUser.ps1 | Update wrapper for agent users with app-only constraints and exit-code handling from the step script. |
| scripts/bulk-agent-registration/Update-A365AgentRegistration.ps1 | Update wrapper for registrations that forwards only bound parameters and blocks invalid step params. |
| scripts/bulk-agent-registration/Update-A365AgentIdentity.ps1 | Update wrapper for agent identities that preserves “only write what was supplied” behavior. |
| scripts/bulk-agent-registration/Remove-A365Blueprint.ps1 | Destructive blueprint delete script with dependent discovery and optional cascade + purge. |
| scripts/bulk-agent-registration/Remove-A365AgentUser.ps1 | Agent user delete script with typed-resource safety checks, planning output, and optional purge. |
| scripts/bulk-agent-registration/Remove-A365AgentRegistration.ps1 | Unregister script for admin-center inventory entries (registrations/instances) with planning + verification. |
| scripts/bulk-agent-registration/Remove-A365AgentIdentity.ps1 | Agent identity delete script with typed-resource safety checks, planning output, and optional purge. |
Review details
Suppressed comments (1)
scripts/bulk-agent-registration/Remove-A365AgentRegistration.ps1:976
- The "Nothing to do" message lists parameters (-AgentIdentityId, -AgentUserId, -BlueprintAppId) that this script does not accept, which can send users looking for non-existent options.
$hasTargeting = $RegistrationId -or $AgentInstanceId -or $Agent
if (-not $hasTargeting) {
throw 'Nothing to do. Supply -RegistrationId, -AgentInstanceId, -AgentIdentityId, -AgentUserId, -BlueprintAppId or -Agent.'
}
- Files reviewed: 10/15 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🔵 Needs a closer look
It introduces large, destructive Graph-deletion scripts and currently contains a confirmed validation bug in Remove-A365AgentRegistration.ps1 that must be fixed.
Review details
Suppressed comments (1)
scripts/bulk-agent-registration/Remove-A365AgentRegistration.ps1:974
- The 'Nothing to do' error message lists parameters (-AgentIdentityId, -AgentUserId, -BlueprintAppId) that this script does not define, which can mislead callers when they hit this validation.
$hasTargeting = $RegistrationId -or $AgentInstanceId -or $Agent
if (-not $hasTargeting) {
throw 'Nothing to do. Supply -RegistrationId, -AgentInstanceId, -AgentIdentityId, -AgentUserId, -BlueprintAppId or -Agent.'
}
- Files reviewed: 10/15 changed files
- Comments generated: 1
- Review effort level: Lite
…//github.com/walterluna/Agent365-devTools into users/walterluna/add-bulk-onboarding-scripts
There was a problem hiding this comment.
🟡 Changes recommended
The Update-* wrapper scripts contain a parsing-breaking $forwardable array definition (missing commas), so they will not run as-is.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (4)
scripts/bulk-agent-registration/Update-A365Blueprint.ps1:138
- $forwardable is missing commas between the newline-separated string literals, which makes the script fail to parse (PowerShell requires ',' between array elements).
'DisplayName', 'Description', 'Sponsor', 'Owner', 'RequiredPermission'
'RequireOwnerAssignment', 'NewClientSecret', 'GrantAdminConsent'
'SkipInheritablePermissions', 'ManagedIdentityPrincipalId'
'ClientId', 'ClientSecret', 'CertificateThumbprint', 'Certificate', 'CertificatePath'
'CertificatePassword', 'UseManagedIdentity', 'AccessToken', 'Interactive', 'SkipPermissionCheck'
scripts/bulk-agent-registration/Update-A365AgentUser.ps1:145
- $forwardable is missing commas between the newline-separated string literals, which makes the script fail to parse.
$forwardable = @(
'DisplayName', 'MailNickname', 'UsageLocation', 'ManagerUserId', 'ManagerUpn'
'AssignLicense', 'LicenseSkuId', 'LicenseSkuPartNumber', 'DisabledPlans'
'ClientId', 'ClientSecret', 'CertificateThumbprint', 'CertificateStoreLocation'
'Certificate', 'CertificatePath', 'CertificatePassword', 'UseManagedIdentity'
'ManagedIdentityClientId', 'AccessToken'
)
scripts/bulk-agent-registration/Update-A365AgentRegistration.ps1:130
- $forwardable is missing commas between the newline-separated string literals, which makes the script fail to parse.
$forwardable = @(
'DisplayName', 'Description', 'Owner', 'OwnerId', 'AgentIdentityId', 'BlueprintAppId'
'SkipDisplayNameNormalization'
'ClientId', 'ClientSecret', 'CertificateThumbprint', 'Certificate', 'CertificatePath'
'CertificatePassword', 'UseManagedIdentity', 'AccessToken', 'Interactive', 'SkipPermissionCheck'
)
scripts/bulk-agent-registration/Update-A365AgentIdentity.ps1:147
- $forwardable is missing commas between the newline-separated string literals, which makes the script fail to parse.
$forwardable = @(
'DisplayName', 'Tag', 'CustomSecurityAttribute', 'SkipCustomSecurityAttributeValidation'
'Sponsor', 'Owner', 'Disabled', 'RequiredPermission', 'RequireOwnerAssignment'
'GrantAdminConsent', 'OutputJsonPath'
'ClientId', 'ClientSecret', 'CertificateThumbprint', 'Certificate', 'CertificatePath'
'CertificatePassword', 'UseManagedIdentity', 'AccessToken', 'Interactive', 'SkipPermissionCheck'
)
- Files reviewed: 23/28 changed files
- Comments generated: 4
- Review effort level: Lite
add missing params description to all bluk onboarding scripts
There was a problem hiding this comment.
🟡 Changes recommended
The new Update-A365*.ps1 wrappers currently type authentication inputs (ClientSecret/AccessToken/CertificatePassword) as strings and contain at least one misleading behavior description, which together can push plain-text secret usage and confuse callers.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 24/28 changed files
- Comments generated: 5
- Review effort level: Lite
There was a problem hiding this comment.
🔵 Needs a closer look
Confirmed edge-case bugs in report path handling (wildcard expansion) and wrapper robustness/conformance need to be fixed before safe adoption.
Review details
Suppressed comments (10)
Previously missed (5) — in code that hasn't changed since the last review.
scripts/bulk-agent-registration/A365-BulkOnboarding.ps1:265
- Write-A365BulkReport uses New-Item with -Path, which treats wildcard characters (e.g. [ ] *) in the output path as patterns. This can break report writing for valid literal paths; use -LiteralPath for filesystem creation just like the CSV importer does.
This issue also appears on line 266 of the same file.
scripts/bulk-agent-registration/Update-A365AgentIdentity.ps1:188
- This wrapper doesn’t enable Set-StrictMode, but the step scripts/orchestrator in this folder do. Enabling StrictMode here improves safety for the wrapper’s parameter-forwarding contract.
This issue also appears on line 217 of the same file.
scripts/bulk-agent-registration/Update-A365AgentRegistration.ps1:165
- This wrapper doesn’t enable Set-StrictMode, while other scripts in this suite do. Adding StrictMode helps ensure the forwarding contract fails fast on typos/uninitialized variables.
This issue also appears on line 191 of the same file.
scripts/bulk-agent-registration/Update-A365AgentUser.ps1:170
- This wrapper doesn’t enable Set-StrictMode, while the rest of the bulk-agent-registration scripts do. Adding StrictMode helps catch typos/uninitialized variables in the wrapper’s argument-forwarding logic.
This issue also appears on line 209 of the same file.
scripts/bulk-agent-registration/Update-A365Blueprint.ps1:170
- This wrapper doesn’t enable Set-StrictMode, while other scripts in this suite do (e.g. New-A365AgentBlueprint.ps1). Adding StrictMode helps catch uninitialized variables/typos in these forwarding-only entry points too.
This issue also appears on line 199 of the same file.
scripts/bulk-agent-registration/A365-BulkOnboarding.ps1:269
- When creating the report file, New-Item is called with -Path, which performs wildcard expansion. Use -LiteralPath so output paths containing characters like [ ] are handled correctly.
if ($WithSecrets) {
New-Item -ItemType File -Path $Path -Force | Out-Null
Protect-A365BulkReportFile -Path $Path
}
scripts/bulk-agent-registration/Update-A365Blueprint.ps1:203
- If a caller explicitly passes -StepParameter $null, this will throw when accessing $StepParameter.Keys. Treat $null as "no extra parameters" so the wrapper remains robust when StepParameter is supplied indirectly (e.g. from a variable that may be null).
foreach ($k in $StepParameter.Keys) {
if ($k -in @('Update', 'BlueprintId', 'TenantId')) {
throw "Do not pass '$k' through -StepParameter; it is supplied by this script."
}
$forward[$k] = $StepParameter[$k]
scripts/bulk-agent-registration/Update-A365AgentUser.ps1:213
- If a caller explicitly passes -StepParameter $null, this will throw when accessing $StepParameter.Keys. Treat $null as an empty hashtable so the wrapper reliably forwards only when extra parameters are actually provided.
foreach ($k in $StepParameter.Keys) {
if ($k -in @('Update', 'AgentUserId', 'TenantId', 'UserPrincipalName')) {
throw "Do not pass '$k' through -StepParameter; it is supplied by this script, and the user principal name cannot be changed after creation."
}
$forward[$k] = $StepParameter[$k]
scripts/bulk-agent-registration/Update-A365AgentIdentity.ps1:221
- If a caller explicitly passes -StepParameter $null, this will throw when accessing $StepParameter.Keys. Treat $null as "no extra parameters" to avoid unexpected NullReference failures.
foreach ($k in $StepParameter.Keys) {
if ($k -in @('Update', 'AgentIdentityId', 'TenantId')) {
throw "Do not pass '$k' through -StepParameter; it is supplied by this script."
}
$forward[$k] = $StepParameter[$k]
scripts/bulk-agent-registration/Update-A365AgentRegistration.ps1:198
- If a caller explicitly passes -StepParameter $null, this will throw when accessing $StepParameter.Keys. Treat $null as an empty set of extra parameters (and keep the ManagedByAppId guard).
foreach ($k in $StepParameter.Keys) {
if ($k -in @('Update', 'RegistrationId', 'TenantId')) {
throw "Do not pass '$k' through -StepParameter; it is supplied by this script."
}
if ($k -eq 'ManagedByAppId') {
throw 'managedByAppId cannot be changed on an existing registration; the service rejects it on PATCH.'
}
$forward[$k] = $StepParameter[$k]
- Files reviewed: 29/33 changed files
- Comments generated: 0 new
- Review effort level: Lite
Krishnadheeraj (DheerajPannala)
left a comment
There was a problem hiding this comment.
LGTM, based on video recoridng
|
Approved & triggered a copilot review if you want to wait for it before merging |
There was a problem hiding this comment.
🟡 Changes recommended
Report security, authentication forwarding, and partial-failure handling contain unresolved correctness issues.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (3)
scripts/bulk-agent-registration/A365-BulkOnboardingCsv.psm1:984
- Stringifying at the recursion cutoff can expose the values this function is meant to redact. A deeply nested
PSCustomObjectcontainingAccessToken = 'TOP-SECRET'is serialized here as@{AccessToken=TOP-SECRET}and then written to the aggregate report. Return a fixed redacted sentinel at the cutoff instead of callingToString()on the remaining object.
if ($Depth -gt 8) { return [string]$Value }
scripts/bulk-agent-registration/A365-BulkOnboarding.ps1:455
- When only
$env:A365_CLIENT_SECRETis set, this counts it as valid authentication but$authSplatcontains only bound parameters, so the secret is never forwarded. The orchestrator repeats that omission, whileNew-A365AgentUser.ps1auto-detects onlyAZURE_CLIENT_SECRET; batches with AgentUser rows therefore pass preflight, create earlier resources, and then fail at the user row. Copy the environment value into the in-process auth splat when the parameter was omitted.
if ($ClientSecret -or $env:A365_CLIENT_SECRET) { $authModes += 'ClientSecret' }
scripts/bulk-agent-registration/A365-BulkOnboarding.ps1:467
- This categorically classifies
AccessTokenas non-app-only, butNew-A365AgentUser.ps1explicitly accepts-AccessTokenand uses it directly; a valid app-only token is therefore rejected before any row runs. Decode the token claims to distinguish application tokens (roles) from delegated tokens (scp), or defer that validation to the leaf script instead of rejecting every access token.
$isAppOnly = $authModes[0] -in @('ClientSecret', 'Certificate', 'ManagedIdentity')
if (-not $isAppOnly -and @($plan.Nodes | Where-Object { $_.ObjectType -eq 'AgentUser' }).Count -gt 0) {
throw "The CSV has AgentUser row(s), which require app-only authentication, but this run authenticates as '$($authModes[0])'. Re-run with -ClientId plus -ClientSecret / -CertificateThumbprint / -UseManagedIdentity."
- Files reviewed: 29/33 changed files
- Comments generated: 4
- Review effort level: Balanced
Adds a Graph-based PowerShell automation suite for provisioning and managing Microsoft Agent 365 resources without requiring the a365 CLI.
Key changes
Security and reliability
Validation