-
Notifications
You must be signed in to change notification settings - Fork 56
Upgrade dependencies to resolve critical vulnerabilities and address Azure DevOps API v15 breaking changes AB#180 #114
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
…g change fixes Co-authored-by: MOlausson <7904771+MOlausson@users.noreply.github.com>
Co-authored-by: MOlausson <7904771+MOlausson@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR upgrades azure-devops-node-api from v6.5.0 to v15.1.1 to resolve 3 critical CVEs and adapt to breaking API changes. Key changes include updating method names, migrating from process-scoped to collection-scoped field APIs, and handling new type system structures. The PR also adds retry logic for network resilience and comprehensive JSDoc documentation.
Key Changes:
- Security: Upgraded dependencies to eliminate critical CVEs in typed-rest-client and underscore
- API Compatibility: Updated all Azure DevOps API calls to v15 method names and structures
- Type Safety: Added explicit return types and removed unsafe type assertions
Reviewed changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| package.json | Upgraded azure-devops-node-api (6.5.0→15.1.1), TypeScript (2.8→5.9), and related dependencies |
| src/common/Utilities.ts | Added WIT↔WITProcessDefinitions converters, retry logic, behavior parent resolution, and URL validation |
| src/common/ProcessExporter.ts | Updated API calls to v15 methods (getListOfProcesses, getProcessByItsId, getProcessBehaviors, getProcessWorkItemTypeRules) |
| src/common/ProcessImporter.ts | Migrated to collection-scoped fields API, updated rule/behavior filtering, added retry handling for API v15 limitations |
| src/common/Engine.ts | Added configurable retry mechanism with exponential backoff for network timeout errors |
| src/common/Interfaces.ts | Changed fields type from FieldModel[] to WorkItemField[], added retry configuration options |
| src/common/Constants.ts | Improved configuration template comments for clarity |
| src/nodejs/Main.ts | Fixed typo in error message, improved comments, added Engine configuration call |
| src/nodejs/NodeJsUtilities.ts | Added JSDoc comments for all public methods |
| src/nodejs/FileLogger.ts | Added JSDoc comments |
| src/nodejs/ConfigurationProcessor.ts | Added JSDoc comments and overwriteProcessOnTarget parameter handling |
| src/common/Logger.ts | Updated JSDoc comment |
| src/common/Errors.ts | Added JSDoc comments for all error classes |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| if (processModel.properties && processModel.properties.parentProcessTypeId) { | ||
| parentProcessTypeId = processModel.properties.parentProcessTypeId; | ||
| } else if ((processModel as any).parentProcessTypeId) { | ||
| parentProcessTypeId = (processModel as any).parentProcessTypeId; | ||
| } else if ((processModel as any).parentTypeId) { | ||
| parentProcessTypeId = (processModel as any).parentTypeId; | ||
| } | ||
|
|
Copilot
AI
Dec 7, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Multiple type assertions with as any bypass TypeScript's type safety. Consider creating a union type or type guard to handle different API response structures more safely.
| if (processModel.properties && processModel.properties.parentProcessTypeId) { | |
| parentProcessTypeId = processModel.properties.parentProcessTypeId; | |
| } else if ((processModel as any).parentProcessTypeId) { | |
| parentProcessTypeId = (processModel as any).parentProcessTypeId; | |
| } else if ((processModel as any).parentTypeId) { | |
| parentProcessTypeId = (processModel as any).parentTypeId; | |
| } | |
| // Type guards for different possible shapes | |
| function hasPropertiesParentProcessTypeId(obj: any): obj is { properties: { parentProcessTypeId: string } } { | |
| return obj && obj.properties && typeof obj.properties.parentProcessTypeId === "string"; | |
| } | |
| function hasParentProcessTypeId(obj: any): obj is { parentProcessTypeId: string } { | |
| return obj && typeof obj.parentProcessTypeId === "string"; | |
| } | |
| function hasParentTypeId(obj: any): obj is { parentTypeId: string } { | |
| return obj && typeof obj.parentTypeId === "string"; | |
| } | |
| if (hasPropertiesParentProcessTypeId(processModel)) { | |
| parentProcessTypeId = processModel.properties.parentProcessTypeId; | |
| } else if (hasParentProcessTypeId(processModel)) { | |
| parentProcessTypeId = processModel.parentProcessTypeId; | |
| } else if (hasParentTypeId(processModel)) { | |
| parentProcessTypeId = processModel.parentTypeId; | |
| } |
| } else if (behavior.inherits && (behavior.inherits as any).behaviorRefName) { | ||
| // New API structure: inherits.behaviorRefName | ||
| inheritsId = (behavior.inherits as any).behaviorRefName; | ||
| } else if ((behavior as any).inheritsId) { | ||
| inheritsId = (behavior as any).inheritsId; | ||
| } else if ((behavior as any).parentId) { | ||
| inheritsId = (behavior as any).parentId; | ||
| } |
Copilot
AI
Dec 7, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Multiple type assertions with as any bypass TypeScript's type safety. Consider creating a union type or type guard to handle different API response structures more safely.
|
|
||
| // Extract behavior ID from various API property structures | ||
| let behaviorId = behavior.id; | ||
| if (!behaviorId || behaviorId === 'undefined' || behaviorId.trim() === '') { |
Copilot
AI
Dec 7, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Checking if behaviorId === 'undefined' compares against the string literal 'undefined', not the undefined value. This should be behaviorId === undefined or remove this check since the previous condition !behaviorId already handles undefined.
| } | ||
|
|
||
| // Final validation - ensure we have a valid behavior ID | ||
| if (!behaviorId || behaviorId === 'undefined' || behaviorId.trim() === '') { |
Copilot
AI
Dec 7, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Duplicate check: checking if behaviorId === 'undefined' compares against the string literal 'undefined', not the undefined value. This should be behaviorId === undefined or remove this check since !behaviorId already handles undefined.
| const behaviorId = behavior.id || (behavior as any).referenceName || (behavior as any).behaviorId || behavior.name; | ||
|
|
||
| // Skip behaviors with invalid IDs | ||
| if (!behaviorId || behaviorId === 'undefined' || behaviorId.trim() === '') { |
Copilot
AI
Dec 7, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Checking if behaviorId === 'undefined' compares against the string literal 'undefined', not the undefined value. This should be behaviorId === undefined or remove this check since !behaviorId already handles undefined.
| if (!behaviorId || behaviorId === 'undefined' || behaviorId.trim() === '') { | |
| if (!behaviorId || behaviorId.trim() === '') { |
| return await Engine.Task(() => this._witProcessApi.getProcesses(), `Get processes on target account`); | ||
| }, () => new ValidationError("Failed to get processes on target acccount, check account url, token and token permission.")); | ||
| return await Engine.Task(() => this._witProcessApi.getListOfProcesses(), `Get processes on target account`); | ||
| }, () => new ValidationError("Failed to get processes on target account, check account url, token and token permission.")); |
Copilot
AI
Dec 7, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The error message uses 'acccount' (with three c's). This appears to be a typo and should be 'account'.
| const fieldExist = fieldsOnTarget.some(targetField => targetField.referenceName === sourceField.referenceName); | ||
| if (!fieldExist) { |
Copilot
AI
Dec 7, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Variable name 'fieldExist' should be 'fieldExists' to follow proper English grammar and naming conventions.
| const fieldExist = fieldsOnTarget.some(targetField => targetField.referenceName === sourceField.referenceName); | |
| if (!fieldExist) { | |
| const fieldExists = fieldsOnTarget.some(targetField => targetField.referenceName === sourceField.referenceName); | |
| if (!fieldExists) { |
…endencies-and-test
Upgrades
azure-devops-node-apifrom 6.5.0 to 15.1.1 to resolve 3 critical CVEs (auth leak in typed-rest-client, RCE in underscore). Includes TypeScript 2.8→5.9 upgrade and adaptation to Azure DevOps API breaking changes.Security Fixes
Azure DevOps API Breaking Changes
Method Renames
Fields API Restructure
Fields moved from process-scoped to collection-scoped API:
Added converters
WITToWITProcessDefinitionsFieldModel()andWITToWITProcessDefinitionsFieldType()to handle type system differences (FieldType enums have different numeric values: String is 0 vs 1).Type System Changes
Interface Updates
FieldRuleModel→ProcessRulewithCreateProcessRuleRequestfor creationfield.id→field.referenceNameproperties.class, now top-levelcustomizationTypeCode Quality
as any)Testing Recommendations
Critical paths requiring validation:
Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.