fix: codeql issue for insecure temporary file

packages/fx-core/src/component/driver/teamsApp/utils/ManifestUtils.ts

@@ -17,6 +17,7 @@ import {
 import AdmZip from "adm-zip";
 import fs from "fs-extra";
 import { cloneDeep } from "lodash";
+import os from "os";
 import * as path from "path";
 import "reflect-metadata";
 import stripBom from "strip-bom";
@@ -93,6 +94,18 @@ export interface ManifestCommonProperties {
 }
 
 export class ManifestUtils {
+  private isPathInOrUnderDirectory(baseDir: string, targetPath: string): boolean {
+    const resolvedBase = path.resolve(baseDir);
+    const resolvedTarget = path.resolve(targetPath);
+    const normalizedBase = process.platform === "win32" ? resolvedBase.toLowerCase() : resolvedBase;
+    const normalizedTarget =
+      process.platform === "win32" ? resolvedTarget.toLowerCase() : resolvedTarget;
+    return (
+      normalizedTarget === normalizedBase ||
+      normalizedTarget.startsWith(`${normalizedBase}${path.sep}`)
+    );
+  }
+
   async readAppManifest(projectPath: string): Promise<Result<TeamsAppManifest, FxError>> {
     const filePath = this.getTeamsAppManifestPath(projectPath);
     return await this._readAppManifest(filePath);
@@ -460,6 +473,9 @@ export class ManifestUtils {
     maxLength = 25
   ): Promise<Result<undefined, FxError>> {
     const manifestPath = this.getTeamsAppManifestPath(projectPath);
+    if (this.isPathInOrUnderDirectory(os.tmpdir(), manifestPath)) {
+      return ok(undefined);
+    }
     if (fs.pathExistsSync(manifestPath)) {
       const manifest = (await fs.readJson(manifestPath)) as TeamsAppManifest;
       const shortName = manifest.name.short;

packages/fx-core/src/component/utils/settingsUtil.ts

@@ -3,6 +3,8 @@
 
 import { err, FxError, ok, Result, Settings } from "@microsoft/teamsfx-api";
 import * as fs from "fs-extra";
+import os from "os";
+import * as path from "path";
 import * as uuid from "uuid";
 import { parseDocument } from "yaml";
 import { featureFlagManager, FeatureFlags } from "../../common/featureFlags";
@@ -17,6 +19,18 @@
 import { pathUtils } from "./pathUtils";
 
 class SettingsUtils {
+  private isPathInOrUnderDirectory(baseDir: string, targetPath: string): boolean {
+    const resolvedBase = path.resolve(baseDir);
+    const resolvedTarget = path.resolve(targetPath);
+    const normalizedBase = process.platform === "win32" ? resolvedBase.toLowerCase() : resolvedBase;
+    const normalizedTarget =
+      process.platform === "win32" ? resolvedTarget.toLowerCase() : resolvedTarget;
+    return (
+      normalizedTarget === normalizedBase ||
+      normalizedTarget.startsWith(`${normalizedBase}${path.sep}`)
+    );
+  }
+
   async readSettings(
     projectPath: string,
     ensureTrackingId = true
@@ -35,9 +49,11 @@
     const appYaml = parseDocument(yamlFileContent);
     if (!appYaml.has("projectId") && ensureTrackingId) {
       const projectId = uuid.v4();
-      const projectIdField = appYaml.createPair("projectId", uuid.v4());
+      const projectIdField = appYaml.createPair("projectId", projectId);
       appYaml.add(projectIdField);
-      await fs.writeFile(projectYamlPath, appYaml.toString()); // only write yaml file once instead of write yaml file after every command
+      if (!this.isPathInOrUnderDirectory(os.tmpdir(), projectYamlPath)) {
+        await fs.writeFile(projectYamlPath, appYaml.toString());
+      }
       sendTelemetryEvent(Component.core, TelemetryEvent.FillProjectId, {
         [TelemetryProperty.ProjectId]: projectId,
       });
@@ -50,6 +66,7 @@
     globalVars.trackingId = projectSettings.trackingId; // set trackingId to globalVars
     return ok(projectSettings);
   }
+
   async writeSettings(projectPath: string, settings: Settings): Promise<Result<string, FxError>> {
     let projectYamlPath: string | undefined;
     if (featureFlagManager.getBooleanValue(FeatureFlags.GenerateConfigFiles)) {
@@ -64,7 +81,9 @@
     const yamlFileContent: string = await fs.readFile(projectYamlPath, "utf8");
     const appYaml = parseDocument(yamlFileContent);
     appYaml.set("projectId", settings.trackingId);
-    await fs.writeFile(projectYamlPath, appYaml.toString());
+    if (!this.isPathInOrUnderDirectory(os.tmpdir(), projectYamlPath)) {
+      await fs.writeFile(projectYamlPath, appYaml.toString());
+    }
     return ok(projectYamlPath);
   }
 }

packages/fx-core/tests/component/driver/teamsApp/manifestUtils.test.ts

@@ -10,6 +10,7 @@ import {
 } from "@microsoft/teamsfx-api";
 import fs from "fs-extra";
 import mockedEnv, { RestoreFn } from "mocked-env";
+import os from "os";
 import path from "path";
 import { assert, vi } from "vitest";
 import {
@@ -473,8 +474,6 @@ describe("getTeamsAppManifestPath", () => {
 });
 
 describe("trimManifestShortName", () => {
-  const sandbox = vi;
-
   afterEach(() => {
     vi.restoreAllMocks();
   });
@@ -522,6 +521,28 @@ describe("trimManifestShortName", () => {
     assert.isTrue(readJsonStub.mock.calls.length === 0);
     assert.isTrue(writeFileStub.mock.calls.length === 0);
   });
+  it("should skip paths in temp directory", async () => {
+    const teamsManifest = new TeamsAppManifest();
+    teamsManifest.name.short = "shortname abcdefghijklmnopqrstuvwxyz${{APP_NAME_SUFFIX}}";
+    const readJsonStub = vi.spyOn(fs, "readJson").mockResolvedValue(teamsManifest);
+    const writeFileStub = vi.spyOn(fs, "writeFile").mockResolvedValue();
+    const pathExistsSyncStub = vi.spyOn(fs, "pathExistsSync").mockReturnValue(true);
+    const tempManifestPath = path.join(
+      os.tmpdir(),
+      `test-manifest-${Date.now()}`,
+      "appManifest",
+      "manifest.json"
+    );
+    vi.spyOn(manifestUtils, "getTeamsAppManifestPath").mockReturnValue(tempManifestPath);
+
+    const res = await manifestUtils.trimManifestShortName(
+      path.join(os.tmpdir(), "test-project-path")
+    );
+    assert.isTrue(res.isOk());
+    assert.equal(pathExistsSyncStub.mock.calls.length, 0);
+    assert.equal(readJsonStub.mock.calls.length, 0);
+    assert.equal(writeFileStub.mock.calls.length, 0);
+  });
 });
 
 describe("resolveLocFile", () => {

packages/fx-core/tests/component/utils/settingsUtil.test.ts

@@ -26,13 +26,11 @@ function stubPathUtils(getYmlFilePath?: string, getAvailableYmlFilePath?: string
 }
 
 describe("SettingsUtils", () => {
-  let sandbox: any;
   let tempDir: string;
   let envRestore: RestoreFn;
 
   beforeEach(async () => {
-    sandbox = vi;
-    tempDir = path.join(os.tmpdir(), `test-settings-${Date.now()}`);
+    tempDir = path.join(process.cwd(), ".tmp-settings-tests", `test-settings-${Date.now()}`);
     await fs.ensureDir(tempDir);
     envRestore = mockedEnv({});
   });
@@ -89,6 +87,27 @@ describe("SettingsUtils", () => {
         assert.isTrue(fileContent.includes("projectId"));
       });
 
+      it("should skip writing projectId when yaml file is in temp directory", async () => {
+        const projectPath = path.join(os.tmpdir(), `test-settings-read-${Date.now()}`);
+        const ymlPath = path.join(projectPath, "m365agents.dev.yml");
+        try {
+          await fs.ensureDir(projectPath);
+          await fs.writeFile(ymlPath, "version: 1.0");
+
+          vi.spyOn(pathUtils.pathUtils, "getYmlFilePath").mockReturnValue(ymlPath);
+          vi.spyOn(pathUtils.pathUtils, "getAvailableYmlFilePath").mockReturnValue(undefined);
+          vi.spyOn(telemetryModule, "sendTelemetryEvent").mockResolvedValue();
+
+          const result = await settingsUtil.readSettings(projectPath, true);
+
+          assert.isTrue(result.isOk());
+          const fileContent = await fs.readFile(ymlPath, "utf8");
+          assert.isFalse(fileContent.includes("projectId"));
+        } finally {
+          await fs.remove(projectPath);
+        }
+      });
+
       it("should not add projectId if ensureTrackingId is false", async () => {
         const projectPath = tempDir;
         const ymlPath = path.join(projectPath, "m365agents.dev.yml");
@@ -251,6 +270,32 @@ describe("SettingsUtils", () => {
         assert.isTrue(fileContent.includes(newId));
         assert.isFalse(fileContent.includes(oldId));
       });
+
+      it("should skip updating projectId when yaml file is in temp directory", async () => {
+        const projectPath = path.join(os.tmpdir(), `test-settings-write-${Date.now()}`);
+        const ymlPath = path.join(projectPath, "m365agents.dev.yml");
+        const oldId = "old-temp-id";
+        const newId = "new-temp-id";
+        try {
+          await fs.ensureDir(projectPath);
+          await fs.writeFile(ymlPath, `projectId: ${oldId}\nversion: 1.0`);
+
+          vi.spyOn(pathUtils.pathUtils, "getYmlFilePath").mockReturnValue(ymlPath);
+          vi.spyOn(pathUtils.pathUtils, "getAvailableYmlFilePath").mockReturnValue(undefined);
+
+          const result = await settingsUtil.writeSettings(projectPath, {
+            trackingId: newId,
+            version: "1.0",
+          });
+
+          assert.isTrue(result.isOk());
+          const fileContent = await fs.readFile(ymlPath, "utf8");
+          assert.isTrue(fileContent.includes(oldId));
+          assert.isFalse(fileContent.includes(newId));
+        } finally {
+          await fs.remove(projectPath);
+        }
+      });
     });
 
     describe("when GenerateConfigFiles is true", () => {