diff --git a/l10n/bundle.l10n.json b/l10n/bundle.l10n.json index 37ed942b7..6b27e37e0 100644 --- a/l10n/bundle.l10n.json +++ b/l10n/bundle.l10n.json @@ -133,6 +133,9 @@ "User cancelled environment switch": "User cancelled environment switch", "User cancelled folder selection": "User cancelled folder selection", "Download failed: {0}": "Download failed: {0}", + "Metadata diff import URI is missing the required 'filePath' query parameter.": "Metadata diff import URI is missing the required 'filePath' query parameter.", + "Metadata diff import URI must reference an absolute file path without '..' segments.": "Metadata diff import URI must reference an absolute file path without '..' segments.", + "Metadata diff import failed: {0}": "Metadata diff import failed: {0}", "Site download cancelled. Authentication is required to proceed.": "Site download cancelled. Authentication is required to proceed.", "Site download cancelled. Correct environment connection is required.": "Site download cancelled. Correct environment connection is required.", "Site download cancelled. No folder selected.": "Site download cancelled. No folder selected.", @@ -435,6 +438,7 @@ "Export Site Comparison": "Export Site Comparison", "Importing comparison...": "Importing comparison...", "Import Site Comparison": "Import Site Comparison", + "Metadata diff file must be a .json file.": "Metadata diff file must be a .json file.", "Invalid file format. The file does not contain valid metadata diff data.": "Invalid file format. The file does not contain valid metadata diff data.", "Unsupported version. This file was created with a newer version of the extension.": "Unsupported version. This file was created with a newer version of the extension.", "This file was exported with a newer version of the extension. Please update your extension to import this file.": "This file was exported with a newer version of the extension. Please update your extension to import this file.", @@ -613,6 +617,12 @@ "Error message when import fails. {0} is the error message." ] }, + "Metadata diff file not found: {0}/Error message when the supplied import file is missing. {0} is the file path.": { + "message": "Metadata diff file not found: {0}", + "comment": [ + "Error message when the supplied import file is missing. {0} is the file path." + ] + }, "Refreshing comparison for {0} ([details](command:microsoft.powerplatform.pages.actionsHub.showOutputChannel \"Show download output\")).../This is a markdown formatting which must persist across translations.": { "message": "Refreshing comparison for {0} ([details](command:microsoft.powerplatform.pages.actionsHub.showOutputChannel \"Show download output\"))...", "comment": [ diff --git a/loc/translations-export/vscode-powerplatform.xlf b/loc/translations-export/vscode-powerplatform.xlf index be35d03ac..c88c12add 100644 --- a/loc/translations-export/vscode-powerplatform.xlf +++ b/loc/translations-export/vscode-powerplatform.xlf @@ -822,6 +822,22 @@ Return to this chat and @powerpages can help you write and edit your website cod Maximum 30 characters allowed + + Metadata diff file must be a .json file. + + + Metadata diff file not found: {0} + Error message when the supplied import file is missing. {0} is the file path. + + + Metadata diff import URI is missing the required 'filePath' query parameter. + + + Metadata diff import URI must reference an absolute file path without '..' segments. + + + Metadata diff import failed: {0} + Microsoft wants your feedback diff --git a/src/client/power-pages/actions-hub/ActionsHubTreeDataProvider.ts b/src/client/power-pages/actions-hub/ActionsHubTreeDataProvider.ts index 02e8c3ac1..d0da31bad 100644 --- a/src/client/power-pages/actions-hub/ActionsHubTreeDataProvider.ts +++ b/src/client/power-pages/actions-hub/ActionsHubTreeDataProvider.ts @@ -386,7 +386,9 @@ export class ActionsHubTreeDataProvider implements vscode.TreeDataProvider importMetadataDiff()), vscode.commands.registerCommand(Constants.Commands.METADATA_DIFF_RESYNC, resyncMetadataDiff(this._pacTerminal, this._context)), MetadataDiffDecorationProvider.getInstance().register(), ReadOnlyContentProvider.getInstance().register() diff --git a/src/client/power-pages/actions-hub/Constants.ts b/src/client/power-pages/actions-hub/Constants.ts index dfbb59ecc..1f9e0c64c 100644 --- a/src/client/power-pages/actions-hub/Constants.ts +++ b/src/client/power-pages/actions-hub/Constants.ts @@ -268,6 +268,7 @@ export const Constants = { METADATA_DIFF_EXPORT_TITLE: vscode.l10n.t("Export Site Comparison"), METADATA_DIFF_IMPORT_PROGRESS: vscode.l10n.t("Importing comparison..."), METADATA_DIFF_IMPORT_TITLE: vscode.l10n.t("Import Site Comparison"), + METADATA_DIFF_IMPORT_NOT_JSON: vscode.l10n.t("Metadata diff file must be a .json file."), METADATA_DIFF_EXPORT_INVALID_FILE: vscode.l10n.t("Invalid file format. The file does not contain valid metadata diff data."), METADATA_DIFF_EXPORT_UNSUPPORTED_VERSION: vscode.l10n.t("Unsupported version. This file was created with a newer version of the extension."), METADATA_DIFF_EXPORT_NEWER_EXTENSION_VERSION: vscode.l10n.t("This file was exported with a newer version of the extension. Please update your extension to import this file."), @@ -519,6 +520,15 @@ export const Constants = { args: [errorMessage], comment: ["Error message when import fails. {0} is the error message."] }), + /** + * Returns the error message when a pre-supplied import file does not exist + */ + METADATA_DIFF_IMPORT_FILE_NOT_FOUND: (filePath: string) => + vscode.l10n.t({ + message: "Metadata diff file not found: {0}", + args: [filePath], + comment: ["Error message when the supplied import file is missing. {0} is the file path."] + }), /** * Returns the progress message when resyncing a site comparison */ diff --git a/src/client/power-pages/actions-hub/handlers/metadata-diff/ImportMetadataDiffHandler.ts b/src/client/power-pages/actions-hub/handlers/metadata-diff/ImportMetadataDiffHandler.ts index 51666bd31..52f9bd42d 100644 --- a/src/client/power-pages/actions-hub/handlers/metadata-diff/ImportMetadataDiffHandler.ts +++ b/src/client/power-pages/actions-hub/handlers/metadata-diff/ImportMetadataDiffHandler.ts @@ -112,26 +112,56 @@ function validateImportData(data: unknown): string | undefined { /** * Imports a metadata diff from a JSON file */ -export async function importMetadataDiff(): Promise { +export async function importMetadataDiff(presuppliedFileUri?: vscode.Uri): Promise { traceInfo(Constants.EventNames.ACTIONS_HUB_METADATA_DIFF_IMPORT_CALLED, { - methodName: importMetadataDiff.name + methodName: importMetadataDiff.name, + source: presuppliedFileUri ? "uri_handler" : "command_palette" }); try { - // Show open dialog first (before progress) - const openUris = await vscode.window.showOpenDialog({ - canSelectMany: false, - filters: { - [Constants.Strings.METADATA_DIFF_EXPORT_FILTER_NAME]: ["json"] - }, - title: Constants.Strings.METADATA_DIFF_IMPORT_TITLE - }); + let fileUri: vscode.Uri; - if (!openUris || openUris.length === 0) { - return; // User cancelled - } + if (presuppliedFileUri) { + // URI-handler path: validate the supplied file before proceeding. + if (!fs.existsSync(presuppliedFileUri.fsPath)) { + vscode.window.showErrorMessage( + Constants.StringFunctions.METADATA_DIFF_IMPORT_FILE_NOT_FOUND(presuppliedFileUri.fsPath) + ); + traceError( + Constants.EventNames.ACTIONS_HUB_METADATA_DIFF_IMPORT_FAILED, + new Error("Pre-supplied file path does not exist"), + { methodName: importMetadataDiff.name, reason: "file_not_found" } + ); + return; + } + if (!presuppliedFileUri.fsPath.toLowerCase().endsWith(".json")) { + vscode.window.showErrorMessage( + Constants.Strings.METADATA_DIFF_IMPORT_NOT_JSON + ); + traceError( + Constants.EventNames.ACTIONS_HUB_METADATA_DIFF_IMPORT_FAILED, + new Error("Pre-supplied file is not .json"), + { methodName: importMetadataDiff.name, reason: "wrong_extension" } + ); + return; + } + fileUri = presuppliedFileUri; + } else { + // Command path: existing file-picker flow (unchanged). + const openUris = await vscode.window.showOpenDialog({ + canSelectMany: false, + filters: { + [Constants.Strings.METADATA_DIFF_EXPORT_FILTER_NAME]: ["json"] + }, + title: Constants.Strings.METADATA_DIFF_IMPORT_TITLE + }); + + if (!openUris || openUris.length === 0) { + return; // User cancelled + } - const fileUri = openUris[0]; + fileUri = openUris[0]; + } // Read and parse the file first to validate before showing progress let importData: IMetadataDiffExport; diff --git a/src/client/test/integration/power-pages/actions-hub/handlers/metadata-diff/ImportMetadataDiffHandler.test.ts b/src/client/test/integration/power-pages/actions-hub/handlers/metadata-diff/ImportMetadataDiffHandler.test.ts index a7e769926..dbee87e95 100644 --- a/src/client/test/integration/power-pages/actions-hub/handlers/metadata-diff/ImportMetadataDiffHandler.test.ts +++ b/src/client/test/integration/power-pages/actions-hub/handlers/metadata-diff/ImportMetadataDiffHandler.test.ts @@ -4,6 +4,7 @@ */ import * as vscode from "vscode"; +import * as fs from "fs"; import { expect } from "chai"; import sinon from "sinon"; import * as TelemetryHelper from "../../../../../../power-pages/actions-hub/TelemetryHelper"; @@ -105,6 +106,67 @@ describe("ImportMetadataDiffHandler", () => { }); }); + describe("importMetadataDiff with a pre-supplied URI", () => { + it("should skip the open dialog when a URI is supplied", async () => { + const existsSyncStub = sandbox.stub(fs, "existsSync").returns(true); + // Short-circuit the read so we don't run the full import flow. + sandbox.stub(fs, "readFileSync").throws(new Error("stop after dialog check")); + + const { importMetadataDiff } = await import("../../../../../../power-pages/actions-hub/handlers/metadata-diff/ImportMetadataDiffHandler"); + + await importMetadataDiff(vscode.Uri.file("/tmp/diff.json")); + + expect(showOpenDialogStub.called).to.be.false; + expect(existsSyncStub.called).to.be.true; + }); + + it("should show an error when the supplied file does not exist", async () => { + sandbox.stub(fs, "existsSync").returns(false); + + const { importMetadataDiff } = await import("../../../../../../power-pages/actions-hub/handlers/metadata-diff/ImportMetadataDiffHandler"); + + await importMetadataDiff(vscode.Uri.file("/tmp/missing.json")); + + expect(showOpenDialogStub.called).to.be.false; + expect(showErrorMessageStub.calledOnce).to.be.true; + expect(showErrorMessageStub.firstCall.args[0]).to.match(/not found/i); + }); + + it("should show an error when the supplied file is not a .json file", async () => { + sandbox.stub(fs, "existsSync").returns(true); + + const { importMetadataDiff } = await import("../../../../../../power-pages/actions-hub/handlers/metadata-diff/ImportMetadataDiffHandler"); + + await importMetadataDiff(vscode.Uri.file("/tmp/diff.txt")); + + expect(showOpenDialogStub.called).to.be.false; + expect(showErrorMessageStub.calledOnce).to.be.true; + expect(showErrorMessageStub.firstCall.args[0]).to.match(/\.json/i); + }); + + it("should record the URI handler source in telemetry", async () => { + const traceInfoStub = TelemetryHelper.traceInfo as sinon.SinonStub; + sandbox.stub(fs, "existsSync").returns(false); + + const { importMetadataDiff } = await import("../../../../../../power-pages/actions-hub/handlers/metadata-diff/ImportMetadataDiffHandler"); + + await importMetadataDiff(vscode.Uri.file("/tmp/missing.json")); + + expect(traceInfoStub.firstCall.args[0]).to.equal("ActionsHubMetadataDiffImportCalled"); + expect(traceInfoStub.firstCall.args[1]).to.include({ source: "uri_handler" }); + }); + + it("should still open the dialog when no URI is supplied", async () => { + showOpenDialogStub.resolves(undefined); + + const { importMetadataDiff } = await import("../../../../../../power-pages/actions-hub/handlers/metadata-diff/ImportMetadataDiffHandler"); + + await importMetadataDiff(); + + expect(showOpenDialogStub.calledOnce).to.be.true; + }); + }); + describe("IMetadataDiffExport format", () => { it("should support new format with localWebsiteId and remoteWebsiteId", () => { // This test verifies that the interface supports the new field names diff --git a/src/client/test/unit/uriHandler.test.ts b/src/client/test/unit/uriHandler.test.ts index 1fbadef5c..bbb021613 100644 --- a/src/client/test/unit/uriHandler.test.ts +++ b/src/client/test/unit/uriHandler.test.ts @@ -4,6 +4,7 @@ */ import { expect } from "chai"; +import { resolveMetadataDiffImportFilePath } from "../../uriHandler/utils/metadataDiffImportValidation"; // Test URI handling functionality describe('UriHandler Schema Parameter Tests', () => { @@ -211,3 +212,47 @@ describe('UriHandler Schema Parameter Tests', () => { }); }); }); + +describe('Metadata Diff Import filePath validation', () => { + it('reports "missing" when filePath is null', () => { + const result = resolveMetadataDiffImportFilePath(null); + expect(result.ok).to.be.false; + expect(result).to.deep.equal({ ok: false, reason: "missing" }); + }); + + it('reports "missing" when filePath is undefined', () => { + const result = resolveMetadataDiffImportFilePath(undefined); + expect(result).to.deep.equal({ ok: false, reason: "missing" }); + }); + + it('reports "missing" when filePath is an empty string', () => { + const result = resolveMetadataDiffImportFilePath(""); + expect(result).to.deep.equal({ ok: false, reason: "missing" }); + }); + + it('reports "invalid" for a relative path', () => { + const result = resolveMetadataDiffImportFilePath("relative/path/diff.json"); + expect(result).to.deep.equal({ ok: false, reason: "invalid" }); + }); + + it('reports "invalid" for a path containing ".." segments', () => { + const result = resolveMetadataDiffImportFilePath("/Users/test/../../etc/diff.json"); + expect(result).to.deep.equal({ ok: false, reason: "invalid" }); + }); + + it('reports "invalid" when ".." is hidden behind URL-encoding', () => { + // %2E%2E decodes to "..", so traversal attempts must be caught after decoding. + const result = resolveMetadataDiffImportFilePath("%2F..%2Fetc%2Fdiff.json"); + expect(result).to.deep.equal({ ok: false, reason: "invalid" }); + }); + + it('accepts an absolute path and returns the decoded path', () => { + const result = resolveMetadataDiffImportFilePath("/Users/test/diff.json"); + expect(result).to.deep.equal({ ok: true, filePath: "/Users/test/diff.json" }); + }); + + it('decodes percent-encoded absolute paths', () => { + const result = resolveMetadataDiffImportFilePath("%2FUsers%2Ftest%20user%2Fdiff.json"); + expect(result).to.deep.equal({ ok: true, filePath: "/Users/test user/diff.json" }); + }); +}); diff --git a/src/client/uriHandler/constants/uriConstants.ts b/src/client/uriHandler/constants/uriConstants.ts index 00330e7d8..9f6ccf33a 100644 --- a/src/client/uriHandler/constants/uriConstants.ts +++ b/src/client/uriHandler/constants/uriConstants.ts @@ -10,7 +10,8 @@ export const URI_CONSTANTS = { EXTENSION_ID: 'microsoft-IsvExpTools.powerplatform-vscode', PATHS: { PCF_INIT: '/pcfInit', - OPEN: '/open' + OPEN: '/open', + METADATA_DIFF_IMPORT: '/metadataDiffImport' }, PARAMETERS: { WEBSITE_ID: 'websiteid', @@ -20,7 +21,8 @@ export const URI_CONSTANTS = { SITE_NAME: 'sitename', WEBSITE_NAME: 'websitename', SITE_URL: 'siteurl', - WEBSITE_PREVIEW_URL: 'websitepreviewurl' + WEBSITE_PREVIEW_URL: 'websitepreviewurl', + FILE_PATH: 'filePath' }, SCHEMA_VALUES: { PORTAL_SCHEMA_V2: 'portalschemav2' @@ -40,4 +42,5 @@ export const URI_CONSTANTS = { export const enum UriPath { PcfInit = '/pcfInit', Open = '/open', + MetadataDiffImport = '/metadataDiffImport', } diff --git a/src/client/uriHandler/constants/uriStrings.ts b/src/client/uriHandler/constants/uriStrings.ts index e83b88254..9e70e2ff6 100644 --- a/src/client/uriHandler/constants/uriStrings.ts +++ b/src/client/uriHandler/constants/uriStrings.ts @@ -19,7 +19,10 @@ export const URI_HANDLER_STRINGS = { ENV_SWITCH_FAILED: vscode.l10n.t("Failed to switch to the required environment. Please sign in with an account that has access to the target environment using 'pac auth create' command."), USER_CANCELLED_ENV_SWITCH: vscode.l10n.t("User cancelled environment switch"), USER_CANCELLED_FOLDER_SELECTION: vscode.l10n.t("User cancelled folder selection"), - DOWNLOAD_FAILED: vscode.l10n.t("Download failed: {0}") + DOWNLOAD_FAILED: vscode.l10n.t("Download failed: {0}"), + METADATA_DIFF_IMPORT_MISSING_FILE_PATH: vscode.l10n.t("Metadata diff import URI is missing the required 'filePath' query parameter."), + METADATA_DIFF_IMPORT_INVALID_FILE_PATH: vscode.l10n.t("Metadata diff import URI must reference an absolute file path without '..' segments."), + METADATA_DIFF_IMPORT_FAILED: vscode.l10n.t("Metadata diff import failed: {0}") }, INFO: { DOWNLOAD_CANCELLED_AUTH: vscode.l10n.t("Site download cancelled. Authentication is required to proceed."), diff --git a/src/client/uriHandler/telemetry/uriHandlerTelemetryEvents.ts b/src/client/uriHandler/telemetry/uriHandlerTelemetryEvents.ts index 40339b871..2404cf066 100644 --- a/src/client/uriHandler/telemetry/uriHandlerTelemetryEvents.ts +++ b/src/client/uriHandler/telemetry/uriHandlerTelemetryEvents.ts @@ -16,5 +16,6 @@ export enum uriHandlerTelemetryEventNames { URI_HANDLER_DOWNLOAD_STARTED = "UriHandlerDownloadStarted", URI_HANDLER_DOWNLOAD_COMPLETED = "UriHandlerDownloadCompleted", URI_HANDLER_FOLDER_OPENED = "UriHandlerFolderOpened", - URI_HANDLER_PCF_INIT_TRIGGERED = "UriHandlerPcfInitTriggered" + URI_HANDLER_PCF_INIT_TRIGGERED = "UriHandlerPcfInitTriggered", + URI_HANDLER_METADATA_DIFF_IMPORT_TRIGGERED = "UriHandlerMetadataDiffImportTriggered" } diff --git a/src/client/uriHandler/uriHandler.ts b/src/client/uriHandler/uriHandler.ts index 96bd60b8f..62ffc8c64 100644 --- a/src/client/uriHandler/uriHandler.ts +++ b/src/client/uriHandler/uriHandler.ts @@ -6,10 +6,12 @@ import * as vscode from "vscode"; import { PacWrapper } from "../pac/PacWrapper"; import { oneDSLoggerWrapper } from "../../common/OneDSLoggerTelemetry/oneDSLoggerWrapper"; -import { UriPath } from "./constants/uriConstants"; +import { UriPath, URI_CONSTANTS } from "./constants/uriConstants"; import { URI_HANDLER_STRINGS } from "./constants/uriStrings"; import { uriHandlerTelemetryEventNames } from "./telemetry/uriHandlerTelemetryEvents"; import { UriHandlerUtils, UriParameters } from "./utils/uriHandlerUtils"; +import { resolveMetadataDiffImportFilePath } from "./utils/metadataDiffImportValidation"; +import { importMetadataDiff } from "../power-pages/actions-hub/handlers/metadata-diff/ImportMetadataDiffHandler"; export function RegisterUriHandler(pacWrapper: PacWrapper): vscode.Disposable { const uriHandler = new UriHandler(pacWrapper); @@ -32,6 +34,56 @@ class UriHandler implements vscode.UriHandler { return this.pcfInit(); } else if (uri.path === UriPath.Open) { return this.handleOpenPowerPages(uri); + } else if (uri.path === UriPath.MetadataDiffImport) { + return this.handleMetadataDiffImport(uri); + } + } + + // vscode://microsoft-IsvExpTools.powerplatform-vscode/metadataDiffImport?filePath= + async handleMetadataDiffImport(uri: vscode.Uri): Promise { + const startTime = Date.now(); + try { + const params = new URLSearchParams(uri.query); + const rawFilePath = params.get(URI_CONSTANTS.PARAMETERS.FILE_PATH); + + oneDSLoggerWrapper.getLogger().traceInfo( + uriHandlerTelemetryEventNames.URI_HANDLER_METADATA_DIFF_IMPORT_TRIGGERED, + { timestamp: startTime.toString(), hasFilePath: rawFilePath ? "true" : "false" } + ); + + const pathResult = resolveMetadataDiffImportFilePath(rawFilePath); + if (!pathResult.ok) { + const message = pathResult.reason === "missing" + ? URI_HANDLER_STRINGS.ERRORS.METADATA_DIFF_IMPORT_MISSING_FILE_PATH + : URI_HANDLER_STRINGS.ERRORS.METADATA_DIFF_IMPORT_INVALID_FILE_PATH; + vscode.window.showErrorMessage(message); + oneDSLoggerWrapper.getLogger().traceInfo( + uriHandlerTelemetryEventNames.URI_HANDLER_METADATA_DIFF_IMPORT_TRIGGERED, + { success: "false", reason: pathResult.reason, duration: (Date.now() - startTime).toString() } + ); + return; + } + + const fileUri = vscode.Uri.file(pathResult.filePath); + await importMetadataDiff(fileUri); + + oneDSLoggerWrapper.getLogger().traceInfo( + uriHandlerTelemetryEventNames.URI_HANDLER_METADATA_DIFF_IMPORT_TRIGGERED, + { success: "true", duration: (Date.now() - startTime).toString() } + ); + } catch (error) { + oneDSLoggerWrapper.getLogger().traceError( + uriHandlerTelemetryEventNames.URI_HANDLER_METADATA_DIFF_IMPORT_TRIGGERED, + "Metadata diff import via URI failed", + error instanceof Error ? error : new Error(String(error)), + { duration: (Date.now() - startTime).toString() } + ); + vscode.window.showErrorMessage( + URI_HANDLER_STRINGS.ERRORS.METADATA_DIFF_IMPORT_FAILED.replace( + "{0}", + error instanceof Error ? error.message : String(error) + ) + ); } } diff --git a/src/client/uriHandler/utils/metadataDiffImportValidation.ts b/src/client/uriHandler/utils/metadataDiffImportValidation.ts new file mode 100644 index 000000000..27fa92b73 --- /dev/null +++ b/src/client/uriHandler/utils/metadataDiffImportValidation.ts @@ -0,0 +1,45 @@ +/* + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + */ + +import * as path from "path"; + +/** + * Result of validating the `filePath` query parameter of a + * `/metadataDiffImport` URI. + * + * Kept free of any `vscode` dependency so the validation logic can be + * exercised by the plain-mocha unit test runner (which cannot load the + * `vscode` runtime module). + */ +export type MetadataDiffImportPathResult = + | { ok: true; filePath: string } + | { ok: false; reason: "missing" | "invalid" }; + +/** + * Validates and decodes the `filePath` supplied on a metadata diff import URI. + * + * Defense-in-depth: only absolute paths without `..` segments are accepted. + * Relative paths and path-traversal attempts are rejected. The user is + * implicitly trusting whoever generated the URL — but we shouldn't make path + * tricks easy. + * + * @param rawFilePath The raw, still-encoded value of the `filePath` query parameter. + * @returns `{ ok: true, filePath }` with the decoded absolute path, or + * `{ ok: false, reason }` describing why it was rejected. + */ +export function resolveMetadataDiffImportFilePath( + rawFilePath: string | null | undefined +): MetadataDiffImportPathResult { + if (!rawFilePath) { + return { ok: false, reason: "missing" }; + } + + const decoded = decodeURIComponent(rawFilePath); + if (!path.isAbsolute(decoded) || decoded.includes("..")) { + return { ok: false, reason: "invalid" }; + } + + return { ok: true, filePath: decoded }; +}