Skip to content

fix: harden workspace output-path validation  - #8423

Draft
Treicy Sanchez (Treicysg) wants to merge 6 commits into
mainfrom
dev/treicys/outputValidation
Draft

Treicy Sanchez (Treicysg) wants to merge 6 commits into
mainfrom
dev/treicys/outputValidation

Conversation

@Treicysg

Copy link
Copy Markdown
Contributor

No description provided.

Copilot AI balanced review requested due to automatic review settings October 16, 2026 20:40
@github-actions

Copy link
Copy Markdown
Contributor

Thanks for opening this pull request! Please link it to an issue by adding the fixes syntax to the pull request description, for example: Fixes #1234.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Remaining containment bypasses can allow cleanup to delete external files or workspace-root contents.

2 open findings
What changed in this PR

Hardens Kiota workspace output-path validation to protect files during generation cleanup.

Changes:

  • Adds symbolic-link and junction containment checks.
  • Revalidates output paths immediately before cleanup.
  • Adds link-validation and external-file preservation tests.
File Description
tests/​Kiota.Builder.Tests/​WorkspaceManagement/​WorkspaceConfigurationStorageServiceTests.cs Tests link containment and cleanup protection.
src/​Kiota.Builder/​WorkspaceManagement/​BaseApiConsumerConfiguration.cs Adds filesystem-aware output-path validation.
src/​Kiota.Builder/​KiotaBuilder.cs Validates workspace output paths before deletion.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/Kiota.Builder/WorkspaceManagement/BaseApiConsumerConfiguration.cs Outdated
Comment thread src/Kiota.Builder/WorkspaceManagement/BaseApiConsumerConfiguration.cs Outdated
Copilot AI balanced review requested due to automatic review settings October 16, 2026 21:48

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

Unresolved cleanup issues and cross-platform link handling around destructive operations require fixes and human verification.

2 open issues
2 resolved since last review

Review effort: Balanced

Comment thread src/Kiota.Builder/KiotaBuilder.cs Outdated
Comment thread src/Kiota.Builder/KiotaBuilder.cs Outdated
Copilot AI balanced review requested due to automatic review settings October 16, 2026 23:14

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Missing output directories and generation without cleanup bypass early validation, allowing writes outside the workspace.

1 open issue
2 resolved since last review

Review effort: Balanced

Comment on lines 83 to +89
if (config.CleanOutput && Directory.Exists(config.OutputPath))
{
if (useKiotaConfig && !config.NoWorkspace)
{
var relativeOutput = Path.GetRelativePath(workspaceDirectory, Path.GetFullPath(config.OutputPath));
BaseApiConsumerConfiguration.ValidateOutputPath(relativeOutput, workspaceDirectory);
}
Copilot AI balanced review requested due to automatic review settings October 22, 2026 22:24

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

Cross-platform link resolution protects destructive deletion and needs human review with runtime validation on Windows and Unix.

1 open issue

Review effort: Balanced

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

2 participants