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):