Skip to content

Conversation

@mikeharder
Copy link
Member

@mikeharder mikeharder commented Aug 30, 2025

// Licensed under the MIT License. See License.txt in the project root for license information.

import * as asyncFs from "@ts-common/fs"
import * as jsonParser from "@ts-common/json-parser"
Copy link
Member Author

Choose a reason for hiding this comment

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

unrelated unused import

@mikeharder mikeharder marked this pull request as ready for review August 30, 2025 01:27
@mikeharder mikeharder requested review from raych1 and scbedd August 30, 2025 01:28
@mikeharder mikeharder changed the title Replace exec with execFile [openApiDiff.ts] Replace exec with execFile Aug 30, 2025
@mikeharder mikeharder self-assigned this Aug 30, 2025
@mikeharder mikeharder moved this from 🤔 Triage to 🔬 Dev in PR in Azure SDK EngSys 📆🎇 Aug 30, 2025
@mikeharder mikeharder requested a review from Copilot August 30, 2025 01:37

This comment was marked as outdated.

@mikeharder mikeharder requested a review from Copilot August 30, 2025 01:40
Copy link

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

This PR replaces the use of child_process.exec() with execFile() to improve security by eliminating shell command injection vulnerabilities. The change removes the need for manual shell argument escaping and simplifies command execution.

  • Replaced exec with execFile in openApiDiff.ts for safer command execution
  • Removed shell-quote dependency and related escaping logic
  • Restructured command building to use argument arrays instead of shell strings

Reviewed Changes

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

File Description
src/lib/validators/openApiDiff.ts Replaced exec with execFile, removed shell escaping functions, restructured command building
src/test/shellEscapingTest.ts Removed entire test file as shell escaping is no longer needed
package.json Removed shell-quote dependency and its type definitions
CHANGELOG.md Added entry documenting the change to execFile

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

mikeharder and others added 2 commits August 29, 2025 22:02
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@mikeharder mikeharder merged commit b680ac8 into Azure:main Sep 2, 2025
7 checks passed
@mikeharder mikeharder deleted the exec-file-2 branch September 2, 2025 15:56
@weshaggard weshaggard moved this from 🔬 Dev in PR to 🎊 Closed in Azure SDK EngSys 📆🎇 Sep 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

Path strings should be quoted or have whitespace escaped

2 participants