fix: codeql issue for insecure temporary file#16173
Conversation
qinzhouxu
requested review from
HuihuiWu-Microsoft,
qfai,
tecton and
wh-alice
as code owners
❌ VscUse Test Plan — Tests unknownWhy these tests: Changed files are in packages/fx-core (core utility/security fix, not vscode-extension or templates), which is an infrastructure/non-template change, so only 2 smoke plans are selected as a basic sanity check. Branch diff: Plans run:
ℹ️ How were these tests selected?GitHub Copilot (Claude Sonnet 4.6, high reasoning) analysed the PR title, description, and the diff between |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| appYaml.add(projectIdField); | ||
| await fs.writeFile(projectYamlPath, appYaml.toString()); // only write yaml file once instead of write yaml file after every command | ||
| if (!this.isInTempDirectory(projectYamlPath)) { | ||
| await fs.writeFile(projectYamlPath, appYaml.toString()); |
| appYaml.set("projectId", settings.trackingId); | ||
| await fs.writeFile(projectYamlPath, appYaml.toString()); | ||
| if (!this.isInTempDirectory(projectYamlPath)) { | ||
| await fs.writeFile(projectYamlPath, appYaml.toString()); |
E2E Test Selection — AI SelectedWhy these tests: Changes to ManifestUtils.ts (teamsApp driver) and settingsUtil.ts in fx-core map to teamsApp/, declarativeAgent/, and feature/multienv* per the fx-core rule; ManifestUtils is directly exercised by teamsApp and declarativeAgent scenarios, and settingsUtil affects multi-environment flows. Cases selected (10):
Need to run more tests?Comment on this PR:
Then re-run the workflow. |
…ile' Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
❌ VscUse Test Plan — Tests unknownWhy these tests: Changed files are in packages/fx-core (core utility/driver code, not templates or vscode-extension UI), which is a cross-cutting infrastructure fix; applying 2 smoke plans as a basic sanity check. Branch diff: Plans run:
ℹ️ How were these tests selected?GitHub Copilot (Claude Sonnet 4.6, high reasoning) analysed the PR title, description, and the diff between |
…ile' Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
❌ VscUse Test Plan — Tests failureWhy these tests: Changed files are in packages/fx-core (core utility/infra fixes for insecure temp file, not VS Code extension or templates), so only 2 smoke plans are selected as a basic sanity check. Branch diff: Plans run:
ℹ️ How were these tests selected?GitHub Copilot (Claude Sonnet 4.6, high reasoning) analysed the PR title, description, and the diff between |
|
@copilot fix the code ql issue in this PR |
❌ VscUse Test Plan — Tests unknownWhy these tests: Changes are in packages/fx-core (not vscode-extension or templates), fixing a CodeQL insecure temp file issue — this is a trivial/infra security fix with no template or VS Code UI impact, so only 2 smoke plans are selected as a basic sanity check. Branch diff: Plans run:
ℹ️ How were these tests selected?GitHub Copilot (Claude Sonnet 4.6, high reasoning) analysed the PR title, description, and the diff between |
|
@copilot there is still code ql scanning for this issue, your fix did not resolve it, fix it |
❌ VscUse Test Plan — Tests unknownWhy these tests: Changed files are in packages/fx-core (not vscode-extension or templates), fixing a CodeQL security issue in utility files — this is an infrastructure/core fix with no direct VS Code UI feature mapping, so only 2 smoke plans are selected as a basic sanity check. Branch diff: Plans run:
ℹ️ How were these tests selected?GitHub Copilot (Claude Sonnet 4.6, high reasoning) analysed the PR title, description, and the diff between |
Addressed in commit 794611b. I tightened the temp-directory containment checks for both settings and manifest write guards (resolved + case-normalized path checks) and fixed the related unit tests to verify early return with no filesystem writes on temp paths. |
❌ VscUse Test Plan — Tests failureWhy these tests: Changed files are in packages/fx-core (core utilities, not vscode-extension), fixing a CodeQL security issue with no template or VS Code UI changes — treated as trivial/infra fix warranting only 2 smoke plans. Branch diff: Results: ✅ 0 passed · ❌ 2 failed (of 2 plans) Plans run:
ℹ️ How were these tests selected?GitHub Copilot (Claude Sonnet 4.6, high reasoning) analysed the PR title, description, and the diff between |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## dev #16173 +/- ##
==========================================
- Coverage 87.27% 87.26% -0.01%
==========================================
Files 633 633
Lines 33182 33198 +16
Branches 7756 7765 +9
==========================================
+ Hits 28959 28971 +12
Misses 2402 2402
- Partials 1821 1825 +4
🚀 New features to boost your workflow:
|
https://dev.azure.com/msazure/Microsoft%20Teams%20Extensibility/_workitems/edit/38459560