From ad55969d26ea7466d130c001c1cb946f1d578cf0 Mon Sep 17 00:00:00 2001 From: marky224 Date: Fri, 1 May 2026 22:14:45 -0500 Subject: [PATCH] Wire Report action buttons, harden /api/analyze errors, enrich HAR picker MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Item 1 — Report actions: Re-run and Export-markdown were rendering but non-functional. Re-run now re-fires whichever input produced the current Report (composer state, uploaded HAR, fixture id, captured DevTools entry). Export-markdown copies a TS-port of core.render.markdown to the clipboard with a transient "Copied" indicator. Share is hidden in v1 — persistence is out of scope; comment in ReportView gates re-enabling it. Item 2 — Extension on certain targets: backend pipeline traced end-to-end with simulated chrome.devtools.network.Request payloads (rich + minimal) against the production Lambda — all produced the expected auth.missing finding for httpbin/401, so no fix shipped for that specific symptom. The trace did surface a real category bug: malformed HAR entries (missing request.method/url, non-integer/out-of-range response.status) were escaping as KeyError or Pydantic ValidationError and yielding silent 500s. parse_har now validates required request fields up front and the Lambda handler catches both ValueError and KeyError, so the user sees an actionable 400 detail instead. Regression tests cover the previous silent-500 paths plus the chrome.devtools.network.Request-shaped 401 end-to-end. expired.badssl.com is now a named example in the extension README's known-limitations section. Item 3 — HAR picker meta: post-upload display now also shows uncompressed file size, capture date (log.pages[0].startedDateTime preferred, entries[0].startedDateTime fallback), and unique-host count. Fields omit cleanly when absent. Strip-banner placement unchanged. CHANGELOG [Unreleased] reflects all three. Co-Authored-By: Claude Opus 4.7 (1M context) --- CHANGELOG.md | 10 ++ deploy/lambda/handler.py | 8 +- extension/README.md | 12 +- extension/src/App.test.tsx | 92 ++++++++++++ extension/src/App.tsx | 4 +- frontend/src/components/FixtureBrowser.tsx | 9 +- frontend/src/components/HarUpload.test.tsx | 105 +++++++++++++ frontend/src/components/HarUpload.tsx | 74 ++++++++- frontend/src/components/ReportView.test.tsx | 60 +++++++- frontend/src/components/ReportView.tsx | 85 ++++++++++- .../src/components/RequestComposer.test.tsx | 20 +++ frontend/src/components/RequestComposer.tsx | 4 +- frontend/src/lib/markdown.ts | 140 ++++++++++++++++++ src/api_medic/core/parser.py | 24 ++- tests/unit/test_lambda_handler.py | 50 +++++++ tests/unit/test_parser.py | 123 +++++++++++++++ 16 files changed, 791 insertions(+), 29 deletions(-) create mode 100644 frontend/src/lib/markdown.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index e6e7ee5..58c5c0d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,16 @@ All notable changes to api-medic are documented here. The format follows [Keep a ## [Unreleased] +### Added + +- HAR picker meta line on the hosted demo's HAR tab now surfaces uncompressed file size, capture date (from `log.pages[0].startedDateTime` or the first entry's `startedDateTime`), and unique-host count alongside the existing entry count. Fields are omitted when absent from the parsed HAR. +- Report action bar: `Re-run` re-fires whichever input produced the current Report (Run-tab composer state, HAR-tab uploaded file, Demos-tab fixture, or extension panel's captured DevTools entry). `Export markdown` copies a markdown rendering of the Report to the clipboard, mirroring the CLI's `--output markdown` format. + +### Fixed + +- Report action buttons (`Re-run`, `Export markdown`) are now wired up — they previously rendered but did nothing on click. `Share report` is intentionally hidden in v1: it requires persistence, which is out of scope. +- Lambda `/api/analyze` now returns 400 with a useful `detail` for malformed HAR entries (missing `request.method`, missing `request.url`, non-integer `response.status`, out-of-range status codes) instead of a silent 500. The HAR parser also validates required request fields up front rather than letting `KeyError` escape. This was hitting any browser-extension capture path that produced a partial entry — the panel previously surfaced "Analyze failed: 500 Internal Server Error" with no actionable hint. + ## [1.1.0] - 2026-05-01 ### Added diff --git a/deploy/lambda/handler.py b/deploy/lambda/handler.py index 2f324f7..51e5617 100644 --- a/deploy/lambda/handler.py +++ b/deploy/lambda/handler.py @@ -73,6 +73,10 @@ def _handle_analyze(event: dict[str, Any]) -> dict[str, Any]: return _err(400, "Body must include 'kind' (one of 'har', 'curl').") kind = body["kind"] + # Catch ValueError (incl. Pydantic ValidationError, which subclasses it) + # and KeyError. Either can be raised deep inside parse_har / analyze when + # an input field is missing or malformed. Without this the Lambda would + # 500 on payloads it could meaningfully reject as 400. try: if kind == "har": har_payload = body.get("har") @@ -86,10 +90,10 @@ def _handle_analyze(event: dict[str, Any]) -> dict[str, Any]: captured = parse_curl(curl) else: return _err(400, f"Unknown kind: {kind!r} (expected 'har' or 'curl').") - except ValueError as e: + report = analyze(captured) + except (ValueError, KeyError) as e: return _err(400, str(e)) - report = analyze(captured) return { "statusCode": 200, "headers": {"Content-Type": "application/json"}, diff --git a/extension/README.md b/extension/README.md index 776c76e..0b917b7 100644 --- a/extension/README.md +++ b/extension/README.md @@ -89,11 +89,13 @@ changes. - The panel only sees requests the browser actually completes. Connections the browser refuses at its own layer — expired or - untrusted certs, mixed-content blocks, CSP-blocked subresources — - never fire `chrome.devtools.network.onRequestFinished`, so they - don't appear in the panel and can't be analyzed. For diagnosing - those, capture a curl reproduction or paste a HAR into the hosted - demo at `https://api-medic.markandrewmarquez.com`. + untrusted certs (e.g. `https://expired.badssl.com`), mixed-content + blocks, CSP-blocked subresources — never fire + `chrome.devtools.network.onRequestFinished`, so they don't appear in + the panel and can't be analyzed. For diagnosing those, capture a + curl reproduction (`curl -v https://expired.badssl.com`) and run + `api-medic from-curl` locally, or paste the failing request into + the hosted demo at `https://api-medic.markandrewmarquez.com`. - Captures are scoped to the page DevTools is attached to. Background tabs, service-worker requests, and requests issued before DevTools was opened won't appear. diff --git a/extension/src/App.test.tsx b/extension/src/App.test.tsx index 28c8c85..3f57366 100644 --- a/extension/src/App.test.tsx +++ b/extension/src/App.test.tsx @@ -151,6 +151,25 @@ describe("", () => { ).toBeInTheDocument(); }); + it("Re-run on the rendered Report re-fires analyzeHarEntry with the same captured entry", async () => { + mockAnalyze.mockResolvedValue(HEALTHY_REPORT); + render(); + const entry = fireRequest(); + + fireEvent.click(screen.getByText(entry.request.url)); + fireEvent.click( + screen.getByRole("button", { name: /Analyze with api-medic/i }), + ); + await screen.findByText(/https:\/\/api\.example\.com\/v1\/health/); + expect(mockAnalyze).toHaveBeenCalledTimes(1); + + fireEvent.click(screen.getByRole("button", { name: /^Re-run$/ })); + await waitFor(() => { + expect(mockAnalyze).toHaveBeenCalledTimes(2); + }); + expect(mockAnalyze.mock.calls[1]?.[0]).toBe(entry); + }); + it("shows an error banner and no report when analyze fails", async () => { mockAnalyze.mockRejectedValue(new Error("Analyze failed: URL is invalid")); render(); @@ -215,6 +234,79 @@ describe("", () => { expect(screen.queryByText(/^Selected:/)).not.toBeInTheDocument(); }); + it("renders the auth.missing finding for a captured 401 DevTools entry", async () => { + // Regression for the reported "extension fails on httpbin/401" symptom. + // We feed the panel a DevTools-shaped 401 entry and assert that when + // the analyze call returns the contractually correct Report (the same + // shape the parser+engine produces server-side, see + // tests/unit/test_parser.py::test_chrome_devtools_401_entry_yields_auth_missing), + // the panel renders the `auth.missing` finding's title — confirming + // the render path doesn't drop the finding. + const httpbin401Report: Report = { + schema_version: "1.0", + source: "extension", + timestamp: "2026-05-01T12:00:00Z", + request: { + method: "GET", + url: "https://httpbin.org/status/401", + headers: { Accept: "*/*" }, + body_size_bytes: 0, + }, + response: { + status_code: 401, + status_text: "UNAUTHORIZED", + headers: { "WWW-Authenticate": 'Basic realm="Fake Realm"' }, + body_size_bytes: 0, + protocol: "HTTP/1.1", + }, + timing: { dns_ms: null, connect_ms: null, tls_ms: null, ttfb_ms: 100, download_ms: 50, total_ms: 150 }, + findings: [ + { + id: "auth.missing", + severity: "critical", + title: "No Authorization header sent", + explanation: + "The server returned 401 and the request didn't include an Authorization header.", + evidence: { status_code: 401, had_authorization_header: false }, + suggested_fix: "Add an Authorization header and retry.", + }, + ], + }; + mockAnalyze.mockResolvedValue(httpbin401Report); + + render(); + fireRequest({ + request: { + method: "GET", + url: "https://httpbin.org/status/401", + headers: [{ name: "Accept", value: "*/*" }], + } as unknown as chrome.devtools.network.Request["request"], + response: { + status: 401, + statusText: "UNAUTHORIZED", + headers: [ + { name: "WWW-Authenticate", value: 'Basic realm="Fake Realm"' }, + ], + content: { size: 0, mimeType: "text/html" }, + } as unknown as chrome.devtools.network.Request["response"], + }); + + fireEvent.click(screen.getByText("https://httpbin.org/status/401")); + fireEvent.click( + screen.getByRole("button", { name: /Analyze with api-medic/i }), + ); + + expect( + await screen.findByText(/No Authorization header sent/), + ).toBeInTheDocument(); + // And the analyze call received the full DevTools entry (not stripped). + const passedEntry = mockAnalyze.mock.calls[0]?.[0] as + | chrome.devtools.network.Request + | undefined; + expect(passedEntry?.request.url).toBe("https://httpbin.org/status/401"); + expect(passedEntry?.response?.status).toBe(401); + }); + it("caps the captured list at 100 entries", () => { const { container } = render(); for (let i = 0; i < 105; i++) { diff --git a/extension/src/App.tsx b/extension/src/App.tsx index 2cc9cef..190d389 100644 --- a/extension/src/App.tsx +++ b/extension/src/App.tsx @@ -128,7 +128,9 @@ export function App() { )} - {report && } + {report && ( + + )} ); } diff --git a/frontend/src/components/FixtureBrowser.tsx b/frontend/src/components/FixtureBrowser.tsx index b9f009b..77bfda9 100644 --- a/frontend/src/components/FixtureBrowser.tsx +++ b/frontend/src/components/FixtureBrowser.tsx @@ -21,6 +21,7 @@ export function FixtureBrowser({ fixtures }: FixtureBrowserProps) { : (fixtures[0]?.id ?? DEFAULT_FIXTURE); const [selected, setSelected] = useState(initial); const [state, setState] = useState({ kind: "idle" }); + const [reloadTick, setReloadTick] = useState(0); useEffect(() => { let cancelled = false; @@ -40,7 +41,7 @@ export function FixtureBrowser({ fixtures }: FixtureBrowserProps) { return () => { cancelled = true; }; - }, [selected]); + }, [selected, reloadTick]); return (
@@ -73,7 +74,11 @@ export function FixtureBrowser({ fixtures }: FixtureBrowserProps) { {state.message}
) : ( - + setReloadTick((n) => n + 1)} + rerunBusy={false} + /> )} ); diff --git a/frontend/src/components/HarUpload.test.tsx b/frontend/src/components/HarUpload.test.tsx index 9007997..ad765d2 100644 --- a/frontend/src/components/HarUpload.test.tsx +++ b/frontend/src/components/HarUpload.test.tsx @@ -77,6 +77,111 @@ describe("HarUpload", () => { ).toBeInTheDocument(); }); + it("Re-run on the rendered Report re-fires /api/analyze with the same file", async () => { + let calls = 0; + vi.spyOn(globalThis, "fetch").mockImplementation(async (input) => { + const url = typeof input === "string" ? input : (input as Request).url; + expect(url.endsWith("/api/analyze")).toBe(true); + calls++; + return new Response(JSON.stringify(corsReport), { status: 200 }); + }); + + render(); + uploadFile( + new File([JSON.stringify(VALID_HAR)], "session.har", { + type: "application/json", + }), + ); + await screen.findByText(/1 entry/); + fireEvent.click(screen.getByRole("button", { name: /^Analyze$/ })); + await screen.findByText(/CORS preflight/); + expect(calls).toBe(1); + + fireEvent.click(screen.getByRole("button", { name: /^Re-run$/ })); + await screen.findByText(/CORS preflight/); + expect(calls).toBe(2); + }); + + it("shows size, capture date, and unique host count when present in the HAR", async () => { + const har = { + log: { + version: "1.2", + creator: { name: "test", version: "0" }, + pages: [ + { id: "p1", startedDateTime: "2026-04-29T15:00:00Z", title: "x" }, + ], + entries: [ + { + request: { method: "GET", url: "https://api.example.com/a" }, + response: { status: 200 }, + }, + { + request: { method: "GET", url: "https://cdn.example.com/b" }, + response: { status: 200 }, + }, + { + request: { method: "GET", url: "https://api.example.com/c" }, + response: { status: 200 }, + }, + ], + }, + }; + render(); + uploadFile( + new File([JSON.stringify(har)], "session.har", { + type: "application/json", + }), + ); + await screen.findByText(/3 entries/); + // Size, capture date, and unique host count appear on the meta line. + expect( + screen.getByText(/captured 2026-04-29T15:00:00Z/), + ).toBeInTheDocument(); + // Two unique hosts (api.example.com, cdn.example.com). + expect(screen.getByText(/2 hosts/)).toBeInTheDocument(); + }); + + it("falls back to entries[0].startedDateTime when log.pages is absent", async () => { + const har = { + log: { + version: "1.2", + creator: { name: "test", version: "0" }, + entries: [ + { + request: { method: "GET", url: "https://api.example.com/a" }, + response: { status: 200 }, + startedDateTime: "2026-04-29T16:30:00Z", + }, + ], + }, + }; + render(); + uploadFile( + new File([JSON.stringify(har)], "session.har", { + type: "application/json", + }), + ); + await screen.findByText(/1 entry/); + expect( + screen.getByText(/captured 2026-04-29T16:30:00Z/), + ).toBeInTheDocument(); + expect(screen.getByText(/1 host/)).toBeInTheDocument(); + }); + + it("omits the capture-date fragment when no startedDateTime is available anywhere", async () => { + render(); + // VALID_HAR has no log.pages and no entries[0].startedDateTime. + uploadFile( + new File([JSON.stringify(VALID_HAR)], "session.har", { + type: "application/json", + }), + ); + await screen.findByText(/1 entry/); + expect(screen.queryByText(/captured /)).toBeNull(); + // Size and host count still render. + expect(screen.getByText(/1 host/)).toBeInTheDocument(); + }); + it("forwards the HAR JSON in the analyze body", async () => { let body: { kind?: string; har?: unknown } = {}; vi.spyOn(globalThis, "fetch").mockImplementation(async (_input, init) => { diff --git a/frontend/src/components/HarUpload.tsx b/frontend/src/components/HarUpload.tsx index ca5e94a..2578fda 100644 --- a/frontend/src/components/HarUpload.tsx +++ b/frontend/src/components/HarUpload.tsx @@ -8,6 +8,9 @@ interface HarFileSummary { name: string; entryCount: number; firstUrl: string | null; + sizeBytes: number; + capturedAt: string | null; + uniqueHostCount: number; } function summarizeHar(name: string, raw: string): HarFileSummary { @@ -20,13 +23,69 @@ function summarizeHar(name: string, raw: string): HarFileSummary { ) { throw new Error("File is not a HAR archive (missing 'log' property)."); } - const log = (parsed as { log: { entries?: unknown } }).log; + const log = (parsed as { + log: { entries?: unknown; pages?: unknown }; + }).log; if (!Array.isArray(log.entries)) { throw new Error("HAR is missing 'log.entries' array."); } - const entries = log.entries as Array<{ request?: { url?: string } }>; + const entries = log.entries as Array<{ + request?: { url?: string }; + startedDateTime?: string; + }>; const firstUrl = entries[0]?.request?.url ?? null; - return { name, entryCount: entries.length, firstUrl }; + const sizeBytes = new TextEncoder().encode(raw).length; + + let capturedAt: string | null = null; + const pages = log.pages; + if (Array.isArray(pages)) { + const p0 = pages[0] as { startedDateTime?: unknown } | undefined; + if (p0 && typeof p0.startedDateTime === "string") { + capturedAt = p0.startedDateTime; + } + } + if (!capturedAt && typeof entries[0]?.startedDateTime === "string") { + capturedAt = entries[0].startedDateTime; + } + + const hosts = new Set(); + for (const e of entries) { + const u = e?.request?.url; + if (typeof u === "string") { + try { + hosts.add(new URL(u).host); + } catch { + // Malformed URL — skip; the parser will surface its own error + // when /api/analyze runs. + } + } + } + + return { + name, + entryCount: entries.length, + firstUrl, + sizeBytes, + capturedAt, + uniqueHostCount: hosts.size, + }; +} + +function formatBytes(n: number): string { + if (n < 1024) return `${n} B`; + if (n < 1024 * 1024) return `${(n / 1024).toFixed(1)} kB`; + return `${(n / (1024 * 1024)).toFixed(1)} MB`; +} + +function renderHarMetaLine(summary: HarFileSummary): string { + const parts: string[] = [formatBytes(summary.sizeBytes)]; + if (summary.capturedAt) parts.push(`captured ${summary.capturedAt}`); + if (summary.uniqueHostCount > 0) { + parts.push( + `${summary.uniqueHostCount} ${summary.uniqueHostCount === 1 ? "host" : "hosts"}`, + ); + } + return parts.join(" · "); } function readAsText(file: File): Promise { @@ -113,6 +172,11 @@ export function HarUpload() { {summary.entryCount === 1 ? "entry" : "entries"} {summary.firstUrl ? ` · first: ${summary.firstUrl}` : ""} + {renderHarMetaLine(summary) ? ( +
+ {renderHarMetaLine(summary)} +
+ ) : null} ) : (
@@ -146,7 +210,9 @@ export function HarUpload() { {strip && strip.action !== "passthrough" ? ( ) : null} - {report ? : null} + {report ? ( + + ) : null}
); } diff --git a/frontend/src/components/ReportView.test.tsx b/frontend/src/components/ReportView.test.tsx index 5162776..edd88df 100644 --- a/frontend/src/components/ReportView.test.tsx +++ b/frontend/src/components/ReportView.test.tsx @@ -1,5 +1,5 @@ -import { describe, it, expect } from "vitest"; -import { render, screen, within } from "@testing-library/react"; +import { describe, it, expect, vi } from "vitest"; +import { fireEvent, render, screen, waitFor, within } from "@testing-library/react"; import fs from "node:fs"; import path from "node:path"; import { fileURLToPath } from "node:url"; @@ -106,4 +106,60 @@ describe("ReportView", () => { expect(screen.getByText("exp:")).toBeInTheDocument(); expect(screen.getByText("2026-04-25T11:23:00Z")).toBeInTheDocument(); }); + + it("does not render a Share button (v1 persistence cut)", () => { + const report = loadFixture("02-jwt-expired.json"); + render(); + expect(screen.queryByRole("button", { name: /share/i })).toBeNull(); + }); + + it("Re-run button invokes onRerun when supplied", () => { + const report = loadFixture("02-jwt-expired.json"); + const onRerun = vi.fn(); + render(); + const button = screen.getByRole("button", { name: /^Re-run$/ }); + fireEvent.click(button); + expect(onRerun).toHaveBeenCalledTimes(1); + }); + + it("Re-run button is hidden when no onRerun handler is provided", () => { + const report = loadFixture("02-jwt-expired.json"); + render(); + expect(screen.queryByRole("button", { name: /Re-run/i })).toBeNull(); + }); + + it("Re-run shows a busy label and is disabled while rerunBusy=true", () => { + const report = loadFixture("02-jwt-expired.json"); + render(); + const button = screen.getByRole("button", { name: /Re-running/ }); + expect(button).toBeDisabled(); + }); + + it("Export markdown writes non-empty markdown to the clipboard", async () => { + const report = loadFixture("02-jwt-expired.json"); + const writeText = vi.fn().mockResolvedValue(undefined); + Object.assign(navigator, { clipboard: { writeText } }); + + render(); + fireEvent.click(screen.getByRole("button", { name: /Export markdown/ })); + + await waitFor(() => { + expect(writeText).toHaveBeenCalledTimes(1); + }); + const written = String(writeText.mock.calls[0]?.[0] ?? ""); + expect(written.length).toBeGreaterThan(0); + expect(written).toMatch(/# api-medic/); + expect(written).toMatch(/Bearer token has expired/); + expect(await screen.findByText(/Copied to clipboard/i)).toBeInTheDocument(); + }); + + it("Export markdown surfaces a failure indicator when clipboard write rejects", async () => { + const report = loadFixture("01-healthy.json"); + const writeText = vi.fn().mockRejectedValue(new Error("nope")); + Object.assign(navigator, { clipboard: { writeText } }); + + render(); + fireEvent.click(screen.getByRole("button", { name: /Export markdown/ })); + expect(await screen.findByText(/Copy failed/i)).toBeInTheDocument(); + }); }); diff --git a/frontend/src/components/ReportView.tsx b/frontend/src/components/ReportView.tsx index a263529..3fc2ba3 100644 --- a/frontend/src/components/ReportView.tsx +++ b/frontend/src/components/ReportView.tsx @@ -1,13 +1,55 @@ +import { useEffect, useState } from "react"; import type { Report } from "../lib/types"; +import { renderMarkdown } from "../lib/markdown"; import { HeaderBar } from "./report/HeaderBar"; import { RequestLine } from "./report/RequestLine"; import { MetricsGrid } from "./report/MetricsGrid"; import { TimingWaterfall } from "./report/TimingWaterfall"; import { FindingCard } from "./report/FindingCard"; -export function ReportView({ report }: { report: Report }) { +// Re-run is universally available across surfaces (Run / HAR / Demos / extension). +// In Demos and HAR contexts it is conceptually a re-render of the same input, +// but exposing it everywhere keeps the action bar consistent and avoids per-tab +// component branching. The parent supplies onRerun; if absent the button is +// hidden (e.g., static rendering tests). +// +// Share is intentionally hidden in v1: it requires persistence (storing the +// Report so a share URL can retrieve it later), and persistence is explicitly +// cut from v1 per docs/architecture.md. Re-enable when persistence ships. +export interface ReportViewProps { + report: Report; + onRerun?: () => void | Promise; + rerunBusy?: boolean; + rerunLabel?: string; +} + +export function ReportView({ + report, + onRerun, + rerunBusy = false, + rerunLabel = "Re-run", +}: ReportViewProps) { const findings = report.findings ?? []; const hasCritical = findings.some((f) => f.severity === "critical"); + const [copyState, setCopyState] = useState<"idle" | "copied" | "failed">( + "idle", + ); + + useEffect(() => { + if (copyState === "idle") return; + const t = setTimeout(() => setCopyState("idle"), 2000); + return () => clearTimeout(t); + }, [copyState]); + + const onExport = async () => { + const md = renderMarkdown(report); + try { + await navigator.clipboard.writeText(md); + setCopyState("copied"); + } catch { + setCopyState("failed"); + } + }; return (
@@ -43,22 +85,49 @@ export function ReportView({ report }: { report: Report }) { )} - {/* Phase 3 wires these to share-by-URL / engine re-run / markdown renderer. */} -
- Share report - Re-run - Export markdown +
+ {onRerun ? ( + + {rerunBusy ? "Re-running…" : rerunLabel} + + ) : null} + Export markdown + {copyState === "copied" ? ( + + Copied to clipboard + + ) : copyState === "failed" ? ( + + Copy failed + + ) : null}
); } -function ActionButton({ children }: { children: React.ReactNode }) { +interface ActionButtonProps { + children: React.ReactNode; + onClick?: () => void | Promise; + disabled?: boolean; +} + +function ActionButton({ children, onClick, disabled }: ActionButtonProps) { return ( diff --git a/frontend/src/components/RequestComposer.test.tsx b/frontend/src/components/RequestComposer.test.tsx index 7e86834..bb631ec 100644 --- a/frontend/src/components/RequestComposer.test.tsx +++ b/frontend/src/components/RequestComposer.test.tsx @@ -66,6 +66,26 @@ describe("RequestComposer", () => { }); }); + it("Re-run on the rendered Report re-fires /api/run", async () => { + const fetchMock = vi + .spyOn(globalThis, "fetch") + .mockImplementation(async (input, init) => { + const url = typeof input === "string" ? input : (input as Request).url; + expect(url.endsWith("/api/run")).toBe(true); + expect(init?.method).toBe("POST"); + return new Response(JSON.stringify(jwtReport), { status: 200 }); + }); + + render(); + fireEvent.click(screen.getByRole("button", { name: /^Run$/ })); + await screen.findByText(/Bearer token has expired/); + expect(fetchMock).toHaveBeenCalledTimes(1); + + fireEvent.click(screen.getByRole("button", { name: /^Re-run$/ })); + await screen.findByText(/Bearer token has expired/); + expect(fetchMock).toHaveBeenCalledTimes(2); + }); + it("shows an error when the API responds non-2xx", async () => { vi.spyOn(globalThis, "fetch").mockImplementation(async () => { return new Response(JSON.stringify({ detail: "boom" }), { status: 500 }); diff --git a/frontend/src/components/RequestComposer.tsx b/frontend/src/components/RequestComposer.tsx index 540ec0d..f3eb0e3 100644 --- a/frontend/src/components/RequestComposer.tsx +++ b/frontend/src/components/RequestComposer.tsx @@ -155,7 +155,9 @@ export function RequestComposer() { {error} ) : null} - {report ? : null} + {report ? ( + + ) : null} ); } diff --git a/frontend/src/lib/markdown.ts b/frontend/src/lib/markdown.ts new file mode 100644 index 0000000..9fa559e --- /dev/null +++ b/frontend/src/lib/markdown.ts @@ -0,0 +1,140 @@ +// Markdown renderer for Reports. Mirrors src/api_medic/core/render/markdown.py +// so Export-markdown produces equivalent output across CLI and web/extension +// surfaces. Severity is conveyed by [CRITICAL]/[WARNING]/[INFO] tags rather +// than emoji to render cleanly in GitHub issues, Slack, or email. + +import type { Finding, Report, Severity, TimingBreakdown } from "./types"; + +const SEVERITY_TAG: Record = { + critical: "[CRITICAL]", + warning: "[WARNING]", + info: "[INFO]", +}; + +export function renderMarkdown(report: Report): string { + const parts: string[] = []; + parts.push("# api-medic — diagnostic report"); + parts.push(""); + parts.push( + `\`${report.request.method} ${report.request.url}\` → ${statusStr(report)}`, + ); + parts.push(""); + parts.push(metricsTable(report)); + parts.push(""); + const timing = timingTable(report.timing); + if (timing) { + parts.push("## Timing"); + parts.push(""); + parts.push(timing); + parts.push(""); + } + parts.push("## Findings"); + parts.push(""); + const findings = report.findings ?? []; + if (findings.length === 0) { + parts.push("_No findings — the request looks healthy._"); + } else { + for (const f of findings) { + parts.push(findingBlock(f)); + parts.push(""); + } + } + return parts.join("\n").replace(/\s+$/, "") + "\n"; +} + +function statusStr(report: Report): string { + if (!report.response) return "_no response_"; + return `\`${report.response.status_code} ${report.response.status_text}\``; +} + +function metricsTable(report: Report): string { + const latency = fmtLatency(report.timing.total_ms ?? null); + const body = report.response + ? fmtBytes(report.response.body_size_bytes) + : "—"; + const protocol = report.response ? report.response.protocol : "—"; + const findings = fmtFindingsCount(report.findings ?? []); + return ( + "| Latency | Body | Protocol | Findings |\n" + + "|---------|------|----------|----------|\n" + + `| ${latency} | ${body} | ${protocol} | ${findings} |` + ); +} + +function timingTable(t: TimingBreakdown): string { + const rows: Array<[string, number]> = []; + const phases: Array<[string, number | null | undefined]> = [ + ["DNS", t.dns_ms], + ["Connect", t.connect_ms], + ["TLS", t.tls_ms], + ["TTFB", t.ttfb_ms], + ["Download", t.download_ms], + ]; + for (const [label, val] of phases) { + if (val !== null && val !== undefined) rows.push([label, val]); + } + if (rows.length === 0 && (t.total_ms === null || t.total_ms === undefined)) { + return ""; + } + const out = ["| Phase | Duration |", "|-------|---------:|"]; + for (const [label, val] of rows) { + out.push(`| ${label} | ${val.toFixed(0)} ms |`); + } + if (t.total_ms !== null && t.total_ms !== undefined) { + out.push(`| **Total** | **${t.total_ms.toFixed(0)} ms** |`); + } + return out.join("\n"); +} + +function findingBlock(f: Finding): string { + const lines: string[] = [ + `### ${SEVERITY_TAG[f.severity]} ${f.title}`, + `**\`${f.id}\`**`, + "", + f.explanation, + ]; + if (f.evidence && Object.keys(f.evidence).length > 0) { + lines.push(""); + lines.push("**Evidence:**"); + for (const [k, v] of Object.entries(f.evidence)) { + lines.push(`- \`${k}\`: \`${evidenceValue(v)}\``); + } + } + if (f.suggested_fix) { + lines.push(""); + lines.push(`**Suggested fix:** ${f.suggested_fix}`); + } + return lines.join("\n"); +} + +function evidenceValue(v: unknown): string { + if (typeof v === "string") return v; + if (typeof v === "number" || typeof v === "boolean") return String(v); + if (v === null || v === undefined) return "null"; + return JSON.stringify(v); +} + +function fmtLatency(total: number | null): string { + if (total === null || total === undefined) return "—"; + if (total < 1000) return `${total.toFixed(0)} ms`; + return `${(total / 1000).toFixed(2)} s`; +} + +function fmtBytes(n: number): string { + if (n < 1024) return `${n} B`; + if (n < 1024 * 1024) return `${(n / 1024).toFixed(1)} kB`; + return `${(n / (1024 * 1024)).toFixed(1)} MB`; +} + +function fmtFindingsCount(findings: Finding[]): string { + if (findings.length === 0) return "0"; + const counts: Record = { + critical: 0, + warning: 0, + info: 0, + }; + for (const f of findings) counts[f.severity] += 1; + if (counts.critical) return `${counts.critical} critical`; + if (counts.warning) return `${counts.warning} warning`; + return `${counts.info} info`; +} diff --git a/src/api_medic/core/parser.py b/src/api_medic/core/parser.py index 64cad92..a8bdd26 100644 --- a/src/api_medic/core/parser.py +++ b/src/api_medic/core/parser.py @@ -38,6 +38,15 @@ def parse_har(raw: str | dict[str, Any]) -> CapturedRequest: raise ValueError("HAR entry is missing 'request'.") request = entry["request"] + if not isinstance(request, dict): + raise ValueError("HAR entry's 'request' must be an object.") + method = request.get("method") + if not isinstance(method, str) or not method: + raise ValueError("HAR entry's request is missing 'method'.") + url = request.get("url") + if not isinstance(url, str) or not url: + raise ValueError("HAR entry's request is missing 'url'.") + request_headers = _har_headers(request.get("headers")) body_text = (request.get("postData") or {}).get("text", "") body = body_text.encode("utf-8") if isinstance(body_text, str) and body_text else b"" @@ -52,9 +61,16 @@ def parse_har(raw: str | dict[str, Any]) -> CapturedRequest: if isinstance(resp_body_text, str) and resp_body_text else b"" ) + try: + status_code = int(response_obj["status"]) + except (TypeError, ValueError) as e: + raise ValueError( + f"HAR entry's response.status is not an integer: {response_obj['status']!r}" + ) from e + status_text_raw = response_obj.get("statusText") captured_response = CapturedResponse( - status_code=int(response_obj["status"]), - status_text=str(response_obj.get("statusText", "")), + status_code=status_code, + status_text=str(status_text_raw) if isinstance(status_text_raw, str) else "", headers=resp_headers, body=resp_body, protocol=str(response_obj.get("httpVersion", "HTTP/1.1")), @@ -63,8 +79,8 @@ def parse_har(raw: str | dict[str, Any]) -> CapturedRequest: timing = _timing_from_har(entry.get("timings") or {}) return CapturedRequest( - method=str(request["method"]).upper(), - url=str(request["url"]), + method=method.upper(), + url=url, headers=request_headers, body=body, response=captured_response, diff --git a/tests/unit/test_lambda_handler.py b/tests/unit/test_lambda_handler.py index 6465e95..0a3ad55 100644 --- a/tests/unit/test_lambda_handler.py +++ b/tests/unit/test_lambda_handler.py @@ -96,6 +96,56 @@ def test_invalid_har_400(self): assert result["statusCode"] == 400 assert "HAR" in json.loads(result["body"])["detail"] + def test_missing_request_method_returns_400_not_500(self): + """Regression: chrome.devtools.network entries occasionally arrive + with no 'method' field on certain Chromium navigation paths. The + handler must surface that as an actionable 400 with a useful detail + rather than a silent 500.""" + result = handler.lambda_handler( + self._har_event( + { + "log": { + "version": "1.2", + "entries": [ + { + "request": {"url": "https://x/"}, + "response": {"status": 401}, + }, + ], + }, + }, + ), + None, + ) + assert result["statusCode"] == 400 + assert "method" in json.loads(result["body"])["detail"] + + def test_out_of_range_response_status_returns_400(self): + """Regression: Pydantic ValidationError from the engine's model + construction is a subclass of ValueError and must be caught at the + handler layer so the user sees a 400 with detail, not a 500.""" + result = handler.lambda_handler( + self._har_event( + { + "log": { + "version": "1.2", + "entries": [ + { + "request": { + "method": "GET", + "url": "https://x/", + }, + "response": {"status": -1}, + }, + ], + }, + }, + ), + None, + ) + assert result["statusCode"] == 400 + assert "status" in json.loads(result["body"])["detail"] + class TestAnalyzeCurl: def test_valid_curl_returns_report(self): diff --git a/tests/unit/test_parser.py b/tests/unit/test_parser.py index 626e65b..7942f3c 100644 --- a/tests/unit/test_parser.py +++ b/tests/unit/test_parser.py @@ -115,6 +115,129 @@ def test_method_normalized_to_upper(self): cap = parse_har(har) assert cap.method == "POST" + def test_missing_request_method_raises_value_error(self): + har = { + "log": { + "version": "1.2", + "entries": [ + { + "request": {"url": "https://x/"}, + "response": {"status": 200}, + }, + ], + }, + } + with pytest.raises(ValueError, match="missing 'method'"): + parse_har(har) + + def test_empty_request_method_raises_value_error(self): + har = _minimal_har() + har["log"]["entries"][0]["request"]["method"] = "" + with pytest.raises(ValueError, match="missing 'method'"): + parse_har(har) + + def test_missing_request_url_raises_value_error(self): + har = { + "log": { + "version": "1.2", + "entries": [ + { + "request": {"method": "GET"}, + "response": {"status": 200}, + }, + ], + }, + } + with pytest.raises(ValueError, match="missing 'url'"): + parse_har(har) + + def test_non_dict_request_raises_value_error(self): + har = { + "log": { + "version": "1.2", + "entries": [{"request": "not-an-object"}], + }, + } + with pytest.raises(ValueError, match="'request' must be an object"): + parse_har(har) + + def test_non_integer_response_status_raises_value_error(self): + har = { + "log": { + "version": "1.2", + "entries": [ + { + "request": {"method": "GET", "url": "https://x/"}, + "response": {"status": "not-a-number"}, + }, + ], + }, + } + with pytest.raises(ValueError, match=r"response\.status is not an integer"): + parse_har(har) + + def test_chrome_devtools_401_entry_yields_auth_missing(self): + """Regression: a chrome.devtools.network.Request-shaped 401 entry + (as the browser extension serializes it) round-trips through the + parser and engine to produce the same `auth.missing` finding the + CLI's run subcommand emits for the equivalent request. The + extension's analyze pathway is contractually byte-identical to + the CLI's (per the architectural invariant) — this guards that. + """ + from api_medic.core.engine import analyze + + # Shape mirrors the extension's buildAnalyzePayload output for an + # unauthenticated GET to httpbin.org/status/401. + entry = { + "request": { + "method": "GET", + "url": "https://httpbin.org/status/401", + "headers": [ + {"name": "Accept", "value": "*/*"}, + {"name": "User-Agent", "value": "Mozilla/5.0"}, + ], + "cookies": [], + "queryString": [], + "headersSize": -1, + "bodySize": 0, + "httpVersion": "HTTP/1.1", + }, + "response": { + "status": 401, + "statusText": "UNAUTHORIZED", + "headers": [ + {"name": "WWW-Authenticate", "value": 'Basic realm="Fake Realm"'}, + {"name": "Content-Type", "value": "text/html"}, + {"name": "Content-Length", "value": "0"}, + ], + "cookies": [], + "content": {"size": 0, "mimeType": "text/html"}, + "redirectURL": "", + "headersSize": -1, + "bodySize": 0, + "httpVersion": "HTTP/1.1", + }, + "startedDateTime": "2026-05-01T12:00:00Z", + "time": 150, + "timings": {"send": 0, "wait": 100, "receive": 50}, + "cache": {}, + } + har = { + "log": { + "version": "1.2", + "creator": {"name": "api-medic-extension", "version": "0.1.0"}, + "entries": [entry], + }, + } + + captured = parse_har(har) + assert captured.response is not None + assert captured.response.status_code == 401 + + report = analyze(captured) + ids = [f.id for f in report.findings] + assert "auth.missing" in ids + class TestParseCurl: def test_post_with_data_and_headers(self):