diff --git a/extensions/ql-vscode/CHANGELOG.md b/extensions/ql-vscode/CHANGELOG.md index 83c4cd4c889..385bf6f2574 100644 --- a/extensions/ql-vscode/CHANGELOG.md +++ b/extensions/ql-vscode/CHANGELOG.md @@ -2,6 +2,8 @@ ## [UNRELEASED] +- Show data flow paths of a variant analysis in a new tab + ## 1.8.0 - 9 March 2023 - Send telemetry about unhandled errors happening within the extension. [#2125](https://github.com/github/vscode-codeql/pull/2125) diff --git a/extensions/ql-vscode/src/pure/interface-types.ts b/extensions/ql-vscode/src/pure/interface-types.ts index aa0ff600ce7..2d68b39d2d3 100644 --- a/extensions/ql-vscode/src/pure/interface-types.ts +++ b/extensions/ql-vscode/src/pure/interface-types.ts @@ -449,6 +449,11 @@ export interface CancelVariantAnalysisMessage { t: "cancelVariantAnalysis"; } +export interface ShowDataFlowPathsMessage { + t: "showDataFlowPaths"; + dataFlowPaths: DataFlowPaths; +} + export type ToVariantAnalysisMessage = | SetVariantAnalysisMessage | SetRepoResultsMessage @@ -462,7 +467,8 @@ export type FromVariantAnalysisMessage = | CopyRepositoryListMessage | ExportResultsMessage | OpenLogsMessage - | CancelVariantAnalysisMessage; + | CancelVariantAnalysisMessage + | ShowDataFlowPathsMessage; export interface SetDataFlowPathsMessage { t: "setDataFlowPaths"; diff --git a/extensions/ql-vscode/src/stories/common/CodePaths.stories.tsx b/extensions/ql-vscode/src/stories/common/CodePaths.stories.tsx index 5b68f1c2d77..793c66099d3 100644 --- a/extensions/ql-vscode/src/stories/common/CodePaths.stories.tsx +++ b/extensions/ql-vscode/src/stories/common/CodePaths.stories.tsx @@ -1,7 +1,6 @@ import * as React from "react"; import { ComponentStory, ComponentMeta } from "@storybook/react"; -import { ThemeProvider } from "@primer/react"; import { CodePaths } from "../../view/common"; import type { CodeFlow } from "../../variant-analysis/shared/analysis-result"; @@ -9,13 +8,7 @@ import type { CodeFlow } from "../../variant-analysis/shared/analysis-result"; export default { title: "Code Paths", component: CodePaths, - decorators: [ - (Story) => ( - - - - ), - ], + decorators: [(Story) => ], } as ComponentMeta; const Template: ComponentStory = (args) => ( diff --git a/extensions/ql-vscode/src/stories/data-flow-paths/DataFlowPaths.stories.tsx b/extensions/ql-vscode/src/stories/data-flow-paths/DataFlowPaths.stories.tsx new file mode 100644 index 00000000000..dd1598360c0 --- /dev/null +++ b/extensions/ql-vscode/src/stories/data-flow-paths/DataFlowPaths.stories.tsx @@ -0,0 +1,19 @@ +import * as React from "react"; + +import { ComponentMeta, ComponentStory } from "@storybook/react"; + +import { DataFlowPaths as DataFlowPathsComponent } from "../../view/data-flow-paths/DataFlowPaths"; +import { createMockDataFlowPaths } from "../../../test/factories/variant-analysis/shared/data-flow-paths"; +export default { + title: "Data Flow Paths/Data Flow Paths", + component: DataFlowPathsComponent, +} as ComponentMeta; + +const Template: ComponentStory = (args) => ( + +); + +export const PowerShell = Template.bind({}); +PowerShell.args = { + dataFlowPaths: createMockDataFlowPaths(), +}; diff --git a/extensions/ql-vscode/src/variant-analysis/variant-analysis-view.ts b/extensions/ql-vscode/src/variant-analysis/variant-analysis-view.ts index 6281ce67e76..5651a6158ec 100644 --- a/extensions/ql-vscode/src/variant-analysis/variant-analysis-view.ts +++ b/extensions/ql-vscode/src/variant-analysis/variant-analysis-view.ts @@ -21,12 +21,15 @@ import { } from "../helpers"; import { telemetryListener } from "../telemetry"; import { redactableError } from "../pure/errors"; +import { DataFlowPathsView } from "./data-flow-paths-view"; +import { DataFlowPaths } from "./shared/data-flow-paths"; export class VariantAnalysisView extends AbstractWebview implements VariantAnalysisViewInterface { public static readonly viewType = "codeQL.variantAnalysis"; + private readonly dataFlowPathsView: DataFlowPathsView; public constructor( ctx: ExtensionContext, @@ -36,6 +39,8 @@ export class VariantAnalysisView super(ctx); manager.registerView(this); + + this.dataFlowPathsView = new DataFlowPathsView(ctx); } public async openView() { @@ -154,6 +159,9 @@ export class VariantAnalysisView this.variantAnalysisId, ); break; + case "showDataFlowPaths": + await this.showDataFlows(msg.dataFlowPaths); + break; case "telemetry": telemetryListener?.sendUIInteraction(msg.action); break; @@ -204,4 +212,8 @@ export class VariantAnalysisView ? `${variantAnalysis.query.name} - Variant Analysis Results` : `Variant Analysis ${this.variantAnalysisId} - Results`; } + + private async showDataFlows(dataFlows: DataFlowPaths): Promise { + await this.dataFlowPathsView.showDataFlows(dataFlows); + } } diff --git a/extensions/ql-vscode/src/view/common/CodePaths/CodePaths.tsx b/extensions/ql-vscode/src/view/common/CodePaths/CodePaths.tsx index 90bd3b44e8e..e7bb09f5b1d 100644 --- a/extensions/ql-vscode/src/view/common/CodePaths/CodePaths.tsx +++ b/extensions/ql-vscode/src/view/common/CodePaths/CodePaths.tsx @@ -1,17 +1,13 @@ import * as React from "react"; -import { useRef, useState } from "react"; import styled from "styled-components"; import { VSCodeLink } from "@vscode/webview-ui-toolkit/react"; -import { Overlay, ThemeProvider } from "@primer/react"; - import { AnalysisMessage, CodeFlow, ResultSeverity, } from "../../../variant-analysis/shared/analysis-result"; -import { CodePathsOverlay } from "./CodePathsOverlay"; -import { useTelemetryOnChange } from "../telemetry"; +import { vscode } from "../../vscode-api"; const ShowPathsLink = styled(VSCodeLink)` cursor: pointer; @@ -24,46 +20,27 @@ export type CodePathsProps = { severity: ResultSeverity; }; -const filterIsOpenTelemetry = (v: boolean) => v; - export const CodePaths = ({ codeFlows, ruleDescription, message, severity, }: CodePathsProps) => { - const [isOpen, setIsOpen] = useState(false); - useTelemetryOnChange(isOpen, "code-path-is-open", { - filterTelemetryOnValue: filterIsOpenTelemetry, - }); - - const linkRef = useRef(null); - - const closeOverlay = () => setIsOpen(false); + const onShowPathsClick = () => { + vscode.postMessage({ + t: "showDataFlowPaths", + dataFlowPaths: { + codeFlows, + ruleDescription, + message, + severity, + }, + }); + }; return ( <> - setIsOpen(true)} ref={linkRef}> - Show paths - - {isOpen && ( - - - - - - )} + Show paths ); }; diff --git a/extensions/ql-vscode/src/view/common/CodePaths/CodePathsOverlay.tsx b/extensions/ql-vscode/src/view/common/CodePaths/CodePathsOverlay.tsx deleted file mode 100644 index 7ba11898aec..00000000000 --- a/extensions/ql-vscode/src/view/common/CodePaths/CodePathsOverlay.tsx +++ /dev/null @@ -1,112 +0,0 @@ -import * as React from "react"; -import { useState } from "react"; -import styled from "styled-components"; - -import { - AnalysisMessage, - CodeFlow, - ResultSeverity, -} from "../../../variant-analysis/shared/analysis-result"; -import { useTelemetryOnChange } from "../telemetry"; -import { SectionTitle } from "../SectionTitle"; -import { VerticalSpace } from "../VerticalSpace"; -import { CodeFlowsDropdown } from "./CodeFlowsDropdown"; -import { CodePath } from "./CodePath"; - -const StyledCloseButton = styled.button` - position: absolute; - top: 1em; - right: 4em; - background-color: var(--vscode-editor-background); - color: var(--vscode-editor-foreground); - border: none; - cursor: pointer; - - &:focus-visible { - outline: none; - } -`; - -const OverlayContainer = styled.div` - height: 100%; - width: 100%; - padding: 2em; - position: fixed; - top: 0; - left: 0; - background-color: var(--vscode-editor-background); - color: var(--vscode-editor-foreground); - overflow-y: scroll; -`; - -const CloseButton = ({ onClick }: { onClick: () => void }) => ( - - - -); - -const PathsContainer = styled.div` - display: flex; - justify-content: center; - align-items: center; -`; - -const PathDetailsContainer = styled.div` - padding: 0; - border: 0; -`; - -const PathDropdownContainer = styled.div` - flex-grow: 1; - padding: 0 0 0 0.2em; - border: none; -`; - -type CodePathsOverlayProps = { - codeFlows: CodeFlow[]; - ruleDescription: string; - message: AnalysisMessage; - severity: ResultSeverity; - onClose: () => void; -}; - -export const CodePathsOverlay = ({ - codeFlows, - ruleDescription, - message, - severity, - onClose, -}: CodePathsOverlayProps) => { - const [selectedCodeFlow, setSelectedCodeFlow] = useState(codeFlows[0]); - useTelemetryOnChange(selectedCodeFlow, "code-flow-selected"); - - return ( - - - - {ruleDescription} - - - - - {codeFlows.length} paths available:{" "} - {selectedCodeFlow.threadFlows.length} steps in - - - - - - - - - - - ); -}; diff --git a/extensions/ql-vscode/src/view/common/CodePaths/__tests__/CodePaths.spec.tsx b/extensions/ql-vscode/src/view/common/CodePaths/__tests__/CodePaths.spec.tsx index a45ed417bf7..1ff6135e1f1 100644 --- a/extensions/ql-vscode/src/view/common/CodePaths/__tests__/CodePaths.spec.tsx +++ b/extensions/ql-vscode/src/view/common/CodePaths/__tests__/CodePaths.spec.tsx @@ -18,20 +18,25 @@ describe(CodePaths.name, () => { />, ); - it("renders correctly when unexpanded", () => { + it("renders 'show paths' link", () => { render(); expect(screen.getByText("Show paths")).toBeInTheDocument(); - expect(screen.queryByText("Code snippet text")).not.toBeInTheDocument(); - expect(screen.queryByText("Rule description")).not.toBeInTheDocument(); }); - it("renders correctly when expanded", async () => { + it("posts extension message when 'show paths' link clicked", async () => { render(); await userEvent.click(screen.getByText("Show paths")); - expect(screen.getByText("Code snippet text")).toBeInTheDocument(); - expect(screen.getByText("Rule description")).toBeInTheDocument(); + expect((window as any).vsCodeApi.postMessage).toHaveBeenCalledWith({ + t: "showDataFlowPaths", + dataFlowPaths: { + codeFlows: createMockCodeFlows(), + ruleDescription: "Rule description", + message: createMockAnalysisMessage(), + severity: "Recommendation", + }, + }); }); }); diff --git a/extensions/ql-vscode/src/view/data-flow-paths/DataFlowPaths.tsx b/extensions/ql-vscode/src/view/data-flow-paths/DataFlowPaths.tsx new file mode 100644 index 00000000000..7bccc0bed78 --- /dev/null +++ b/extensions/ql-vscode/src/view/data-flow-paths/DataFlowPaths.tsx @@ -0,0 +1,71 @@ +import * as React from "react"; +import styled from "styled-components"; +import { useState } from "react"; + +import { useTelemetryOnChange } from "../common/telemetry"; +import { CodeFlowsDropdown } from "../common/CodePaths/CodeFlowsDropdown"; +import { SectionTitle, VerticalSpace } from "../common"; +import { CodePath } from "../common/CodePaths/CodePath"; +import { DataFlowPaths as DataFlowPathsDomainModel } from "../../variant-analysis/shared/data-flow-paths"; + +const PathsContainer = styled.div` + display: flex; + justify-content: center; + align-items: center; +`; + +const PathDetailsContainer = styled.div` + padding: 0; + border: 0; +`; + +const PathDropdownContainer = styled.div` + flex-grow: 1; + padding: 0 0 0 0.2em; + border: none; +`; + +export type DataFlowPathsProps = { + dataFlowPaths: DataFlowPathsDomainModel; +}; + +export const DataFlowPaths = ({ + dataFlowPaths, +}: { + dataFlowPaths: DataFlowPathsDomainModel; +}): JSX.Element => { + const [selectedCodeFlow, setSelectedCodeFlow] = useState( + dataFlowPaths.codeFlows[0], + ); + useTelemetryOnChange(selectedCodeFlow, "code-flow-selected"); + + const { codeFlows, ruleDescription, message, severity } = dataFlowPaths; + + return ( + <> + + {ruleDescription} + + + + + {codeFlows.length} paths available:{" "} + {selectedCodeFlow?.threadFlows.length} steps in + + + + + + + + + + ); +}; diff --git a/extensions/ql-vscode/src/view/data-flow-paths/DataFlowPathsView.tsx b/extensions/ql-vscode/src/view/data-flow-paths/DataFlowPathsView.tsx index e4c6960fd7a..de3d490a7f4 100644 --- a/extensions/ql-vscode/src/view/data-flow-paths/DataFlowPathsView.tsx +++ b/extensions/ql-vscode/src/view/data-flow-paths/DataFlowPathsView.tsx @@ -1,18 +1,19 @@ import * as React from "react"; import { useEffect, useState } from "react"; import { ToDataFlowPathsMessage } from "../../pure/interface-types"; -import { DataFlowPaths } from "../../variant-analysis/shared/data-flow-paths"; +import { DataFlowPaths as DataFlowPathsDomainModel } from "../../variant-analysis/shared/data-flow-paths"; +import { DataFlowPaths } from "./DataFlowPaths"; export type DataFlowPathsViewProps = { - dataFlowPaths?: DataFlowPaths; + dataFlowPaths?: DataFlowPathsDomainModel; }; export function DataFlowPathsView({ dataFlowPaths: initialDataFlowPaths, }: DataFlowPathsViewProps): JSX.Element { - const [dataFlowPaths, setDataFlowPaths] = useState( - initialDataFlowPaths, - ); + const [dataFlowPaths, setDataFlowPaths] = useState< + DataFlowPathsDomainModel | undefined + >(initialDataFlowPaths); useEffect(() => { const listener = (evt: MessageEvent) => { @@ -20,6 +21,10 @@ export function DataFlowPathsView({ const msg: ToDataFlowPathsMessage = evt.data; if (msg.t === "setDataFlowPaths") { setDataFlowPaths(msg.dataFlowPaths); + + // Scroll to the top of the page when we're rendering + // new data flow paths. + window.scrollTo(0, 0); } } else { // sanitize origin @@ -38,11 +43,5 @@ export function DataFlowPathsView({ return <>Loading data flow paths; } - // For now, just render the data flows as JSON. - return ( - <> - Loaded -
{JSON.stringify(dataFlowPaths)}
- - ); + return ; } diff --git a/extensions/ql-vscode/src/view/data-flow-paths/__tests__/DataFlowPaths.spec.tsx b/extensions/ql-vscode/src/view/data-flow-paths/__tests__/DataFlowPaths.spec.tsx new file mode 100644 index 00000000000..02efd210590 --- /dev/null +++ b/extensions/ql-vscode/src/view/data-flow-paths/__tests__/DataFlowPaths.spec.tsx @@ -0,0 +1,31 @@ +import * as React from "react"; +import { render as reactRender, screen } from "@testing-library/react"; +import { DataFlowPaths, DataFlowPathsProps } from "../DataFlowPaths"; +import { createMockDataFlowPaths } from "../../../../test/factories/variant-analysis/shared/data-flow-paths"; + +describe(DataFlowPaths.name, () => { + const render = (props: DataFlowPathsProps) => + reactRender(); + + it("renders data flow paths", () => { + const dataFlowPaths = createMockDataFlowPaths(); + + render({ dataFlowPaths }); + + expect(screen.getByText(dataFlowPaths.ruleDescription)).toBeInTheDocument(); + expect( + screen.getByText("1 paths available", { exact: false }), + ).toBeInTheDocument(); + expect( + screen.getByText("3 steps in", { + exact: false, + }), + ).toBeInTheDocument(); + + expect( + screen.getByText("This zip file may have a dangerous path", { + exact: false, + }), + ).toBeInTheDocument(); + }); +}); diff --git a/extensions/ql-vscode/src/view/data-flow-paths/__tests__/DataFlowPathsView.spec.tsx b/extensions/ql-vscode/src/view/data-flow-paths/__tests__/DataFlowPathsView.spec.tsx index da11f818f17..7baefc06cf6 100644 --- a/extensions/ql-vscode/src/view/data-flow-paths/__tests__/DataFlowPathsView.spec.tsx +++ b/extensions/ql-vscode/src/view/data-flow-paths/__tests__/DataFlowPathsView.spec.tsx @@ -4,6 +4,7 @@ import { DataFlowPathsView, DataFlowPathsViewProps, } from "../DataFlowPathsView"; +import { createMockCodeFlows } from "../../../../test/factories/variant-analysis/shared/CodeFlow"; import { createMockDataFlowPaths } from "../../../../test/factories/variant-analysis/shared/data-flow-paths"; describe(DataFlowPathsView.name, () => { @@ -17,8 +18,14 @@ describe(DataFlowPathsView.name, () => { }); it("renders a data flow paths view", () => { - render({ dataFlowPaths: createMockDataFlowPaths() }); + const dataFlowPaths = createMockDataFlowPaths({ + ruleDescription: "Rule description", + codeFlows: createMockCodeFlows(), + }); - expect(screen.getByText("Loaded")).toBeInTheDocument(); + render({ dataFlowPaths }); + + expect(screen.queryByText("Code snippet text")).toBeInTheDocument(); + expect(screen.getByText("Rule description")).toBeInTheDocument(); }); }); diff --git a/extensions/ql-vscode/test/factories/variant-analysis/shared/data-flow-paths.ts b/extensions/ql-vscode/test/factories/variant-analysis/shared/data-flow-paths.ts index 18992421148..9f0ae98ac35 100644 --- a/extensions/ql-vscode/test/factories/variant-analysis/shared/data-flow-paths.ts +++ b/extensions/ql-vscode/test/factories/variant-analysis/shared/data-flow-paths.ts @@ -1,106 +1,122 @@ -import { CodeFlow } from "../../../../src/variant-analysis/shared/analysis-result"; +import { + AnalysisMessage, + CodeFlow, + ResultSeverity, +} from "../../../../src/variant-analysis/shared/analysis-result"; import { DataFlowPaths } from "../../../../src/variant-analysis/shared/data-flow-paths"; -export function createMockDataFlowPaths(): DataFlowPaths { - const codeFlows: CodeFlow[] = [ - { - threadFlows: [ - { - fileLink: { - fileLinkPrefix: - "https://github.com/PowerShell/PowerShell/blob/450d884668ca477c6581ce597958f021fac30bff", - filePath: - "src/System.Management.Automation/help/UpdatableHelpSystem.cs", - }, - codeSnippet: { - startLine: 1260, - endLine: 1260, - text: " string extractPath = Path.Combine(destination, entry.FullName);", - }, - highlightedRegion: { - startLine: 1260, - startColumn: 72, - endLine: 1260, - endColumn: 86, - }, - message: { - tokens: [ - { - t: "text", - text: "access to property FullName : String", - }, - ], - }, +const defaultCodeFlows: CodeFlow[] = [ + { + threadFlows: [ + { + fileLink: { + fileLinkPrefix: + "https://github.com/PowerShell/PowerShell/blob/450d884668ca477c6581ce597958f021fac30bff", + filePath: + "src/System.Management.Automation/help/UpdatableHelpSystem.cs", + }, + codeSnippet: { + startLine: 1260, + endLine: 1260, + text: " string extractPath = Path.Combine(destination, entry.FullName);", + }, + highlightedRegion: { + startLine: 1260, + startColumn: 72, + endLine: 1260, + endColumn: 86, + }, + message: { + tokens: [ + { + t: "text", + text: "access to property FullName : String", + }, + ], + }, + }, + { + fileLink: { + fileLinkPrefix: + "https://github.com/PowerShell/PowerShell/blob/450d884668ca477c6581ce597958f021fac30bff", + filePath: + "src/System.Management.Automation/help/UpdatableHelpSystem.cs", + }, + codeSnippet: { + startLine: 1260, + endLine: 1260, + text: " string extractPath = Path.Combine(destination, entry.FullName);", + }, + highlightedRegion: { + startLine: 1260, + startColumn: 46, + endLine: 1260, + endColumn: 87, }, - { - fileLink: { - fileLinkPrefix: - "https://github.com/PowerShell/PowerShell/blob/450d884668ca477c6581ce597958f021fac30bff", - filePath: - "src/System.Management.Automation/help/UpdatableHelpSystem.cs", - }, - codeSnippet: { - startLine: 1260, - endLine: 1260, - text: " string extractPath = Path.Combine(destination, entry.FullName);", - }, - highlightedRegion: { - startLine: 1260, - startColumn: 46, - endLine: 1260, - endColumn: 87, - }, - message: { - tokens: [ - { - t: "text", - text: "call to method Combine : String", - }, - ], - }, + message: { + tokens: [ + { + t: "text", + text: "call to method Combine : String", + }, + ], }, - { - fileLink: { - fileLinkPrefix: - "https://github.com/PowerShell/PowerShell/blob/450d884668ca477c6581ce597958f021fac30bff", - filePath: - "src/System.Management.Automation/help/UpdatableHelpSystem.cs", - }, - codeSnippet: { - startLine: 1261, - endLine: 1261, - text: " entry.ExtractToFile(extractPath);", - }, - highlightedRegion: { - startLine: 1261, - startColumn: 45, - endLine: 1261, - endColumn: 56, - }, - message: { - tokens: [ - { - t: "text", - text: "access to local variable extractPath", - }, - ], - }, + }, + { + fileLink: { + fileLinkPrefix: + "https://github.com/PowerShell/PowerShell/blob/450d884668ca477c6581ce597958f021fac30bff", + filePath: + "src/System.Management.Automation/help/UpdatableHelpSystem.cs", }, - ], + codeSnippet: { + startLine: 1261, + endLine: 1261, + text: " entry.ExtractToFile(extractPath);", + }, + highlightedRegion: { + startLine: 1261, + startColumn: 45, + endLine: 1261, + endColumn: 56, + }, + message: { + tokens: [ + { + t: "text", + text: "access to local variable extractPath", + }, + ], + }, + }, + ], + }, +]; + +const defaultMessage: AnalysisMessage = { + tokens: [ + { + t: "text", + text: "This zip file may have a dangerous path", }, - ]; + ], +}; +export function createMockDataFlowPaths({ + codeFlows = defaultCodeFlows, + ruleDescription = "ZipSlip vulnerability", + message = defaultMessage, + severity = "Warning", +}: { + codeFlows?: CodeFlow[]; + ruleDescription?: string; + message?: AnalysisMessage; + severity?: ResultSeverity; +} = {}): DataFlowPaths { return { codeFlows, - ruleDescription: "ZipSlip vulnerability", - message: { - tokens: [ - { - t: "text", - text: "This zip file may have a dangerous path", - }, - ], - }, - severity: "Warning", + ruleDescription, + message, + severity, }; }