Skip to content
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

Add option in Arcade to preserve RepoOrigin and insert it into the publish-to-disk path #15572

Merged

Conversation

jkoritzinsky
Copy link
Member

@jkoritzinsky jkoritzinsky commented Feb 24, 2025

Allow publishing to publish packages to a repo-specific subfolder when publishing to disk.

This (in combination with changes in the VMR orchestrator) will allow us to publish a given VMR vertical with the built-in Arcade publishing logic instead of the custom task we have today.

Contributes to dotnet/sdk#47076

To double check:

Copy link

@Copilot 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.

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

{
Directory.CreateDirectory(NonShippingPackagesLocalStorageDir);
CopyFileAsHardLinkIfPossible(path, Path.Combine(NonShippingPackagesLocalStorageDir, filename), true);
packageDestinationPath = Path.Combine(packageDestinationPath, artifactModel.RepoOrigin);
Copy link
Preview

Copilot AI Feb 25, 2025

Choose a reason for hiding this comment

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

Appending artifactModel.RepoOrigin directly to file paths could lead to unintended directory traversal if it contains path separator characters. Consider sanitizing or validating the RepoOrigin value before using it in Path.Combine.

Suggested change
packageDestinationPath = Path.Combine(packageDestinationPath, artifactModel.RepoOrigin);
packageDestinationPath = Path.Combine(packageDestinationPath, SanitizeRepoOrigin(artifactModel.RepoOrigin));

Copilot is powered by AI, so mistakes are possible. Review output carefully before use.

@jkoritzinsky jkoritzinsky requested a review from mmitche March 5, 2025 21:54
@jkoritzinsky jkoritzinsky enabled auto-merge (squash) March 5, 2025 22:29
@jkoritzinsky jkoritzinsky merged commit c222cb8 into dotnet:main Mar 5, 2025
11 checks passed
YuliiaKovalova pushed a commit that referenced this pull request Mar 20, 2025
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.

2 participants