diff --git a/docs/implementation/tools.md b/docs/implementation/tools.md index fa4f38c9..591d11bb 100644 --- a/docs/implementation/tools.md +++ b/docs/implementation/tools.md @@ -34,6 +34,8 @@ The two surfaces overlap on the example-search workflow only; for the overlappin **Unified `search` query syntax.** The `search.query` field is the backend discovery query syntax, not a raw pass-through to a per-source search engine. It supports implicit `AND`, uppercase `OR`, parentheses, unary `-`, quoted phrases, semantic qualifiers (`kind:`, `category:`, `path:`, `lang:`, `name:`, `intent:`), and routing qualifiers (`registry:`, `package:`, `version:`, `repo:`). The backend parses the query once and compiles it per source. Structured `name` and `language` inputs are compiled into `name:` / `lang:` qualifiers and AND-ed with the query before sending. Per-source support, ignored features, and incompatibilities are reported in `sourceStatus`. +**Promoted `warnings[]`.** Noteworthy `sourceStatus` entries — sources reporting `incompatibleQueryFeatures`, `ignoredQueryFeatures`, `incompatibleFilters`, `ignoredFilters`, lifecycle anomalies (`indexingStatus`, `codeIndexState`), or a free-form `note` — are also surfaced as a top-level `warnings: string[]` in the completed/incomplete payloads (and appended after parser warnings inside the `search_status` result block). The structured detail still lives in `sourceStatus`; `warnings[]` is the agent-visible signal that something about execution did not match the request. Mitigates backend issue B5: `sources: ["docs"]` plus a `kind:`/`lang:` qualifier returns `results: []` with the only diagnostic buried inside `sourceStatus[].note`. The text-v1 renderer prints the warnings as a `warnings:` preamble. Implementation in `buildSourceStatusWarnings` (`src/shared/unified-search-response.ts`); remove once the backend surfaces these at the top level itself. + ### `pkg_info` response shape **Hand-crafted JSON envelope.** `pkg_info` returns a lean JSON payload designed for agent token efficiency. Fields that do not add caller value are deliberately omitted. Null scalars are omitted; blocks (`github`, `vulnerabilities`, `downloads`, `recentChanges`) are omitted entirely when they carry no actionable data. `vulnerabilities` is omitted when `total === 0` or missing; when present, severity values include a CVSS-banded `severityLabel` (`critical` ≥9, `high` ≥7, `medium` ≥4, else `low`) for agent convenience. @@ -48,7 +50,9 @@ The two surfaces overlap on the example-search workflow only; for the overlappin **Filter-aware summary.** `min_severity` and `include_withdrawn` are passed straight through to the service. The returned `vulnerabilityCount` reflects the filtered set — there is no client-side filtering and no `summary.filtered` dual-block. Callers wanting the unfiltered view omit the flag. -**Partitioning buckets.** Advisories with `isMalicious: true` count **only** under `summary.bySeverity.malware`; severity bands (`critical`/`high`/`medium`/`low`) count non-malicious advisories with a positive CVSS score; non-malicious advisories with no score count under `summary.bySeverity.unrated`. Every returned advisory lands in exactly one bucket — the client-side guarantee is `MALWARE + crit + high + medium + low + unrated = advisories.length`. The sum also equals `summary.total` whenever the backend keeps `vulnerabilityCount` and `vulnerabilities[]` consistent (the expected case on all shipped registries). The malware bucket sorts to the top of the advisory list regardless of score. The `unrated` bucket ensures the terminal breakdown line reconciles with the header total on Rust / PyPI packages where a non-trivial fraction of advisories ship without a CVSS score. +**Partitioning buckets.** Advisories with `isMalicious: true` count **only** under `summary.bySeverity.malware`; severity bands (`critical`/`high`/`medium`/`low`) count non-malicious advisories with a positive CVSS score; non-malicious advisories with no score count under `summary.bySeverity.unrated`. Every returned advisory lands in exactly one bucket — the client-side guarantee is `MALWARE + crit + high + medium + low + unrated = advisories.length`. The sum also equals `summary.total`, which is re-derived from the deduped advisory list (see below). The malware bucket sorts to the top of the advisory list regardless of score. The `unrated` bucket ensures the terminal breakdown line reconciles with the header total on Rust / PyPI packages where a non-trivial fraction of advisories ship without a CVSS score. + +**Alias-cluster dedup.** Some registries (most visibly Crates) return the GHSA-prefixed and the RUSTSEC-prefixed entry for the same underlying vulnerability as separate advisories, cross-linked via `aliases[]`. The shared envelope builder unions clusters over `id ∪ aliases[]`, picks one canonical advisory per cluster (severity-bearing entries first, then `GHSA-*` over `RUSTSEC-*`, then lexicographic `id` ascending), and merges the rest under the canonical's `aliases`. `affectedRanges`, `fixedIn`, malware/withdrawn flags, and the latest `modifiedAt` are unioned across the cluster; a withdrawal only sticks if every cluster member is withdrawn. `summary.total` and `summary.bySeverity` are recomputed from the deduped list so the partition invariant holds. This is a client-side mitigation for backend issue B3 (https://app.githits.com — eval report 2026-04-28); remove `dedupAdvisoriesByAlias` from `src/shared/package-vulnerabilities-response.ts` once the backend dedups upstream. **Version validation.** `pkg_vulns` accepts canonical package versions only. Tag-style refs with a leading `v` (for example `v4.18.0`) are rejected client-side with `INVALID_ARGUMENT` before the backend call. This avoids the current production backend's unhelpful generic error for that input shape. This is intentionally narrow: proper ecosystem-aware version parsing and typed invalid-version errors belong in the backend, not in ad hoc CLI normalization rules. diff --git a/src/shared/package-vulnerabilities-response.test.ts b/src/shared/package-vulnerabilities-response.test.ts index 7a067349..2fd8a459 100644 --- a/src/shared/package-vulnerabilities-response.test.ts +++ b/src/shared/package-vulnerabilities-response.test.ts @@ -5,6 +5,7 @@ import { buildPackageVulnerabilitiesSuccessPayload, compareVersionsAscending, computeBySeverity, + dedupAdvisoriesByAlias, formatPackageVulnerabilitiesTerminal, vulnSeverityLabel, } from "./package-vulnerabilities-response.js"; @@ -294,6 +295,387 @@ describe("buildPackageVulnerabilitiesSuccessPayload — requestedVersion echo", }); }); +describe("dedupAdvisoriesByAlias — alias-cluster collapse", () => { + it("returns input unchanged when no aliases overlap", () => { + const input = cloneFixture().security?.vulnerabilities ?? []; + const out = dedupAdvisoriesByAlias(input); + expect(out.length).toBe(input.length); + }); + + it("returns empty array on empty input", () => { + expect(dedupAdvisoriesByAlias([])).toEqual([]); + }); + + it("collapses GHSA + RUSTSEC pair sharing a CVE alias", () => { + const out = dedupAdvisoriesByAlias([ + { + osvId: "GHSA-xjxc-vfw2-cg96", + aliases: ["CVE-2018-20997", "RUSTSEC-2018-0010"], + severityScore: 9.8, + publishedAt: "2021-08-25T00:00:00Z", + affectedVersionRanges: [">=0.10.8 <0.10.9"], + fixedInVersions: ["0.10.9"], + }, + { + osvId: "RUSTSEC-2018-0010", + aliases: ["CVE-2018-20997", "GHSA-xjxc-vfw2-cg96"], + publishedAt: "2018-06-01T00:00:00Z", + affectedVersionRanges: [">=0.10.8 <0.10.9"], + fixedInVersions: ["0.10.9"], + }, + ]); + expect(out.length).toBe(1); + const merged = out[0]; + if (!merged) throw new Error("expected one merged advisory"); + // GHSA wins because it carries a positive severity score. + expect(merged.osvId).toBe("GHSA-xjxc-vfw2-cg96"); + // The other id and shared CVE survive as aliases on the canonical. + expect(merged.aliases?.sort()).toEqual([ + "CVE-2018-20997", + "RUSTSEC-2018-0010", + ]); + expect(merged.aliases).not.toContain("GHSA-xjxc-vfw2-cg96"); + expect(merged.severityScore).toBe(9.8); + }); + + it("links chains via shared aliases (transitive merge)", () => { + // Three entries: A↔B share CVE-X; B↔C share CVE-Y. All three end up + // in one cluster because B bridges A and C. + const out = dedupAdvisoriesByAlias([ + { + osvId: "GHSA-aaa", + aliases: ["CVE-X"], + severityScore: 8.0, + }, + { + osvId: "RUSTSEC-2024-A", + aliases: ["CVE-X", "CVE-Y"], + }, + { + osvId: "RUSTSEC-2024-B", + aliases: ["CVE-Y"], + }, + ]); + expect(out.length).toBe(1); + const merged = out[0]; + if (!merged) throw new Error("expected merged advisory"); + expect(merged.osvId).toBe("GHSA-aaa"); + expect(merged.aliases?.sort()).toEqual([ + "CVE-X", + "CVE-Y", + "RUSTSEC-2024-A", + "RUSTSEC-2024-B", + ]); + }); + + it("prefers GHSA over RUSTSEC when neither has severity", () => { + const out = dedupAdvisoriesByAlias([ + { + osvId: "RUSTSEC-2025-0099", + aliases: ["GHSA-zzz"], + }, + { + osvId: "GHSA-zzz", + aliases: ["RUSTSEC-2025-0099"], + }, + ]); + expect(out.length).toBe(1); + expect(out[0]?.osvId).toBe("GHSA-zzz"); + }); + + it("falls back to lexicographic id when prefix and severity tie", () => { + const out = dedupAdvisoriesByAlias([ + { + osvId: "GHSA-bbb", + aliases: ["CVE-Z"], + }, + { + osvId: "GHSA-aaa", + aliases: ["CVE-Z"], + }, + ]); + expect(out[0]?.osvId).toBe("GHSA-aaa"); + }); + + it("unions affectedVersionRanges and fixedInVersions across the cluster", () => { + const out = dedupAdvisoriesByAlias([ + { + osvId: "GHSA-xxx", + aliases: ["CVE-Q"], + severityScore: 7.5, + affectedVersionRanges: [">=1.0.0 <1.1.0"], + fixedInVersions: ["1.1.0"], + }, + { + osvId: "RUSTSEC-2024-X", + aliases: ["CVE-Q"], + affectedVersionRanges: [">=0.9.0 <1.0.0"], + fixedInVersions: ["1.0.0"], + }, + ]); + const merged = out[0]; + if (!merged) throw new Error("expected merged"); + expect(merged.affectedVersionRanges?.sort()).toEqual([ + ">=0.9.0 <1.0.0", + ">=1.0.0 <1.1.0", + ]); + expect(merged.fixedInVersions?.sort()).toEqual(["1.0.0", "1.1.0"]); + }); + + it("inherits malware flag from any cluster member", () => { + const out = dedupAdvisoriesByAlias([ + { + osvId: "GHSA-mal", + aliases: ["CVE-MAL"], + severityScore: 9.0, + }, + { + osvId: "RUSTSEC-MAL", + aliases: ["CVE-MAL"], + isMalicious: true, + }, + ]); + expect(out[0]?.isMalicious).toBe(true); + }); + + it("clears withdrawn flag if any cluster member is still active", () => { + const out = dedupAdvisoriesByAlias([ + { + osvId: "GHSA-aaa", + aliases: ["CVE-W"], + severityScore: 7.0, + }, + { + osvId: "RUSTSEC-W", + aliases: ["CVE-W"], + withdrawnAt: "2024-01-01T00:00:00Z", + }, + ]); + expect(out[0]?.withdrawnAt).toBeUndefined(); + }); + + it("keeps withdrawn flag when every cluster member is withdrawn", () => { + const out = dedupAdvisoriesByAlias([ + { + osvId: "GHSA-aaa", + aliases: ["CVE-W"], + severityScore: 7.0, + withdrawnAt: "2024-01-15T00:00:00Z", + }, + { + osvId: "RUSTSEC-W", + aliases: ["CVE-W"], + withdrawnAt: "2024-01-01T00:00:00Z", + }, + ]); + expect(out[0]?.withdrawnAt).toBe("2024-01-15T00:00:00Z"); + }); + + it("passes singletons (no aliases, no overlap) through unchanged", () => { + const out = dedupAdvisoriesByAlias([ + { osvId: "GHSA-solo", severityScore: 5.0 }, + ]); + expect(out.length).toBe(1); + expect(out[0]?.osvId).toBe("GHSA-solo"); + }); + + it("does not merge advisories that lack ids and aliases", () => { + const out = dedupAdvisoriesByAlias([ + { summary: "ghost A" }, + { summary: "ghost B" }, + ]); + expect(out.length).toBe(2); + }); + + it("merges advisories with no osvId but overlapping aliases", () => { + // Backend occasionally ships advisories with aliases populated but + // no top-level osvId (e.g. legacy CVE-only entries on PyPI). They + // should still cluster on shared aliases. + const out = dedupAdvisoriesByAlias([ + { + aliases: ["CVE-NO-ID"], + severityScore: 6.0, + }, + { + aliases: ["CVE-NO-ID"], + affectedVersionRanges: [">=1.0.0 <2.0.0"], + }, + ]); + expect(out.length).toBe(1); + const merged = out[0]; + if (!merged) throw new Error("expected merged advisory"); + // Canonical pick (severity-bearing wins) keeps the first member's + // shape; aliases on the merged are the union minus the canonical's + // own osvId — but neither has an osvId, so the alias survives. + expect(merged.aliases).toEqual(["CVE-NO-ID"]); + expect(merged.severityScore).toBe(6.0); + expect(merged.affectedVersionRanges).toEqual([">=1.0.0 <2.0.0"]); + }); + + it("does not promote a withdrawn sibling's severity into the merged record", () => { + // The merged advisory survives because the GHSA member is active. + // The RUSTSEC sibling was retracted; its score is no longer + // authoritative and must not inflate the merged band. + const out = dedupAdvisoriesByAlias([ + { + osvId: "GHSA-active", + aliases: ["CVE-W"], + severityScore: 5.0, + }, + { + osvId: "RUSTSEC-2023-W", + aliases: ["CVE-W"], + severityScore: 9.8, + withdrawnAt: "2024-01-15T00:00:00Z", + }, + ]); + const merged = out[0]; + if (!merged) throw new Error("expected merged advisory"); + expect(merged.severityScore).toBe(5.0); + expect(merged.withdrawnAt).toBeUndefined(); + }); + + it("reports the maximum severity across the cluster (not the canonical's)", () => { + // GHSA wins canonical pick on prefix, but RUSTSEC sibling has a + // higher score. Merged should reflect the higher band so the + // bySeverity histogram is conservative. + const out = dedupAdvisoriesByAlias([ + { + osvId: "GHSA-low", + aliases: ["CVE-X"], + severityScore: 5.0, + }, + { + osvId: "RUSTSEC-2025-X", + aliases: ["CVE-X"], + severityScore: 9.5, + }, + ]); + const merged = out[0]; + if (!merged) throw new Error("expected merged advisory"); + expect(merged.osvId).toBe("GHSA-low"); + expect(merged.severityScore).toBe(9.5); + }); + + it("does not promote a sibling's publishedAt to the merged modifiedAt", () => { + // Canonical was last touched 2024-01-15; sibling was published + // 2025-06-01 but has no explicit modifiedAt. The merged record + // should keep the canonical's modifiedAt (or none) — a publish + // date is not a modification date. + const out = dedupAdvisoriesByAlias([ + { + osvId: "GHSA-aaa", + aliases: ["CVE-Y"], + severityScore: 7.0, + publishedAt: "2024-01-01T00:00:00Z", + modifiedAt: "2024-01-15T00:00:00Z", + }, + { + osvId: "RUSTSEC-2025-Y", + aliases: ["CVE-Y"], + publishedAt: "2025-06-01T00:00:00Z", + }, + ]); + expect(out[0]?.modifiedAt).toBe("2024-01-15T00:00:00Z"); + }); + + it("advances modifiedAt when a sibling has a newer real modifiedAt", () => { + const out = dedupAdvisoriesByAlias([ + { + osvId: "GHSA-aaa", + aliases: ["CVE-Z"], + severityScore: 7.0, + publishedAt: "2024-01-01T00:00:00Z", + modifiedAt: "2024-01-15T00:00:00Z", + }, + { + osvId: "RUSTSEC-2025-Z", + aliases: ["CVE-Z"], + modifiedAt: "2025-06-15T00:00:00Z", + }, + ]); + expect(out[0]?.modifiedAt).toBe("2025-06-15T00:00:00Z"); + }); +}); + +describe("buildPackageVulnerabilitiesSuccessPayload — alias-cluster dedup integration", () => { + it("recomputes total and bySeverity from deduped list", () => { + // Two GHSA/RUSTSEC pairs + one solo advisory. Pre-dedup: 5; after: 3. + // Backend `vulnerabilityCount` is intentionally stale (5) so we + // verify the builder re-derives total from the deduped output. + const fixture = { + package: { name: "pkg", registry: "CRATES" as const, version: "0.10.0" }, + security: { + vulnerabilityCount: 5, + currentVersionAffected: true, + upgradePaths: ["0.10.78"], + vulnerabilities: [ + { + osvId: "GHSA-xxx", + aliases: ["CVE-A", "RUSTSEC-2018-0010"], + severityScore: 9.8, + publishedAt: "2021-08-25T00:00:00Z", + }, + { + osvId: "RUSTSEC-2018-0010", + aliases: ["CVE-A"], + publishedAt: "2018-06-01T00:00:00Z", + }, + { + osvId: "GHSA-yyy", + aliases: ["CVE-B", "RUSTSEC-2024-B"], + severityScore: 6.0, + }, + { + osvId: "RUSTSEC-2024-B", + aliases: ["CVE-B"], + }, + { + osvId: "GHSA-solo", + // No CVSS — exercises the `unrated` bucket post-dedup. + }, + ], + }, + }; + + const payload = buildPackageVulnerabilitiesSuccessPayload(fixture); + expect(payload.summary.total).toBe(3); + expect(payload.advisories?.length).toBe(3); + + // Buckets reconcile with deduped total. + const buckets = payload.summary.bySeverity ?? {}; + const sum = + (buckets.malware ?? 0) + + (buckets.critical ?? 0) + + (buckets.high ?? 0) + + (buckets.medium ?? 0) + + (buckets.low ?? 0) + + (buckets.unrated ?? 0); + expect(sum).toBe(3); + expect(payload.summary.bySeverity).toEqual({ + critical: 1, + medium: 1, + unrated: 1, + }); + + // Canonical preference: GHSA-xxx wins over its RUSTSEC counterpart. + const ids = payload.advisories?.map((a) => a.id); + expect(ids).toContain("GHSA-xxx"); + expect(ids).not.toContain("RUSTSEC-2018-0010"); + }); + + it("preserves total when no aliases cluster (express fixture)", () => { + // Sanity: the default 6-advisory fixture has no overlapping aliases, + // so dedup is a no-op and total/bySeverity match the pre-dedup + // expectations from the partition-invariant test. + const payload = buildPackageVulnerabilitiesSuccessPayload( + defaultVulnerabilityReport, + ); + expect(payload.summary.total).toBe(6); + expect(payload.advisories?.length).toBe(6); + }); +}); + describe("formatPackageVulnerabilitiesTerminal", () => { it("renders zero-vulns hot path as header + one summary body line", () => { const output = formatPackageVulnerabilitiesTerminal(zeroVulnsFixture(), { diff --git a/src/shared/package-vulnerabilities-response.ts b/src/shared/package-vulnerabilities-response.ts index 47defc6d..24c0a32b 100644 --- a/src/shared/package-vulnerabilities-response.ts +++ b/src/shared/package-vulnerabilities-response.ts @@ -7,18 +7,27 @@ * - Backend is the single source of truth for counts. `minSeverity` * and `includeWithdrawn` are passed through on the wire; the * backend returns a filter-aware `vulnerabilityCount`. The builder - * does no filtering of its own. + * does no selective filtering of its own — but it does collapse + * alias-clustered duplicates (see below) because that is shape + * normalisation rather than selection. + * - Alias-cluster dedup runs before bucketing. Some registries (most + * visibly Crates) return both the GHSA-prefixed and the + * RUSTSEC-prefixed entry for the same underlying vulnerability; + * `aliases[]` carries the cross-link. The builder unions clusters + * over `id ∪ aliases`, picks one canonical advisory per cluster + * (severity-bearing entries first, then GHSA over RUSTSEC, then + * lexicographic `id`), and merges the rest under the canonical's + * `aliases`. Both `summary.total` and `summary.bySeverity` are + * recomputed from the deduped list — the partition invariant + * (bucket sum equals total) is preserved post-dedup. This is a + * client-side mitigation for backend issue B3; remove once the + * backend dedups upstream. * - Malware bucket is disjoint from severity bands. `summary.bySeverity` * carries a `malware` key counting `isMalicious === true` advisories; * severity bands count non-malicious advisories only. Non-malicious * advisories with no CVSS score fall into a disjoint `unrated` * bucket so every returned advisory is accounted for. The buckets - * always partition `security.vulnerabilities[]`; their sum equals - * `summary.total` whenever the backend keeps its `vulnerabilityCount` - * and `vulnerabilities[]` in sync (the expected case on all shipped - * registries). The builder does not re-derive `total` from the array - * length because `vulnerabilityCount` is filter-aware and may - * legitimately exceed the returned list in paginated futures. + * always partition `security.vulnerabilities[]` after dedup. * - `requestedVersion` surfaces whenever the backend-resolved * `version` differs from the caller's (trimmed) input. `v`-prefix * normalisation is intentionally *not* applied here: the `v4.17.0` @@ -95,13 +104,29 @@ export function buildPackageVulnerabilitiesSuccessPayload( ): LeanVulnerabilityReport { const pkg = report.package; const security = report.security; - const total = security?.vulnerabilityCount ?? 0; + + // Dedup before counting: alias-clustered duplicates (GHSA + RUSTSEC + // pairs on Crates, mainly) collapse to one canonical advisory each. + // `total` and `bySeverity` are derived from the deduped list so the + // partition invariant holds. + // + // This recomputation is only correct while the backend returns the + // entire filtered vulnerability set inline with `vulnerabilityCount`. + // If the backend later paginates the advisory list (delivering a + // page slice while `vulnerabilityCount` keeps the global filtered + // count), `total` here will underreport — revisit at that point and + // either compute from a backend-supplied deduped count or run dedup + // server-side. + const dedupedAdvisories = dedupAdvisoriesByAlias( + security?.vulnerabilities ?? [], + ); + const total = dedupedAdvisories.length; const payload: LeanVulnerabilityReport = { registry: lowerRegistry(pkg.registry), name: pkg.name, version: pkg.version, - summary: buildSummary(total, security), + summary: buildSummary(total, security, dedupedAdvisories), }; const requestedEcho = deriveRequestedVersion( @@ -114,7 +139,7 @@ export function buildPackageVulnerabilitiesSuccessPayload( if (total > 0) { const sortedAdvisories = sortAdvisories( - (security?.vulnerabilities ?? []).map(buildAdvisory), + dedupedAdvisories.map(buildAdvisory), ); if (sortedAdvisories.length > 0) { payload.advisories = sortedAdvisories; @@ -177,6 +202,7 @@ function parseVersionForSort(v: string): { main: number[]; pre?: string } { function buildSummary( total: number, security: VulnerabilityReport["security"], + dedupedAdvisories: readonly VulnerabilityDetail[], ): LeanVulnerabilitySummary { const summary: LeanVulnerabilitySummary = { total }; if (total === 0) return summary; @@ -185,7 +211,7 @@ function buildSummary( summary.affected = security.currentVersionAffected; } - const bySeverity = computeBySeverity(security?.vulnerabilities ?? []); + const bySeverity = computeBySeverity(dedupedAdvisories); const anyCounted = Object.values(bySeverity).some((n) => n > 0); if (anyCounted) { const trimmed: Partial> = {}; @@ -252,6 +278,270 @@ export function vulnSeverityLabel( return "low"; } +// -------------------------------------------------------------------- +// Alias-cluster dedup +// -------------------------------------------------------------------- + +/** + * Collapse advisories that share an `id`/`aliases` identifier into one + * canonical entry per cluster. The merged entry: + * - unions `aliases`, `affectedVersionRanges`, and `fixedInVersions`; + * - reports the **maximum** `severityScore` across the cluster + * (security tooling errs toward the more conservative band when + * OSV and RUSTSEC disagree); + * - tracks the latest `modifiedAt` actually present (no fallback to + * `publishedAt` — a sibling without a real modification timestamp + * does not advance the merged record); + * - inherits the malware flag from any member that carries it; + * - keeps the withdrawn flag only when **every** cluster member is + * withdrawn (a single active member means the vulnerability is + * still tracked under at least one source). + * + * Canonical preference (deterministic, used to pick the entry whose + * non-merged fields — `osvId`, `summary`, `publishedAt` — survive): + * 1. Member with a positive CVSS score wins over a member without. + * 2. Among severity-bearing members (or among score-less members), + * `GHSA-*` ids beat `RUSTSEC-*` and other prefixes. + * 3. Tiebreak on lexicographic `osvId` ascending. + * + * Members without an `osvId` and without aliases are passed through as + * singleton clusters — they cannot link to anything else. Empty input + * returns an empty array. + */ +export function dedupAdvisoriesByAlias( + advisories: readonly VulnerabilityDetail[], +): VulnerabilityDetail[] { + if (advisories.length === 0) return []; + + // Union-find over identifier strings. Each advisory's `osvId` and + // every alias becomes a node; sharing any node merges the clusters. + // Every node passed to `union` is `ensure()`d first, so `parent.get` + // never returns `undefined` for a known id. + const parent = new Map(); + const find = (id: string): string => { + let root = id; + let next = parent.get(root); + while (next !== undefined && next !== root) { + root = next; + next = parent.get(root); + } + // Path compression: walk again, pointing each node directly at root. + let cursor = id; + while (cursor !== root) { + const parentOfCursor = parent.get(cursor); + if (parentOfCursor === undefined) break; + parent.set(cursor, root); + cursor = parentOfCursor; + } + return root; + }; + const union = (a: string, b: string): void => { + const ra = find(a); + const rb = find(b); + if (ra !== rb) parent.set(ra, rb); + }; + const ensure = (id: string): void => { + if (!parent.has(id)) parent.set(id, id); + }; + + // Track the per-advisory primary identifier so we can group entries + // back into their cluster after the union pass. Advisories with + // neither id nor aliases get a synthetic key so they remain a + // singleton cluster. + const advisoryKeys: string[] = []; + for (let i = 0; i < advisories.length; i++) { + const advisory = advisories[i]; + if (!advisory) { + advisoryKeys.push(`__synthetic_${i}`); + continue; + } + const ids: string[] = []; + if (advisory.osvId) ids.push(advisory.osvId); + for (const alias of advisory.aliases ?? []) { + if (alias) ids.push(alias); + } + if (ids.length === 0) { + const synthetic = `__synthetic_${i}`; + ensure(synthetic); + advisoryKeys.push(synthetic); + continue; + } + for (const id of ids) ensure(id); + for (let j = 1; j < ids.length; j++) { + const a = ids[j - 1]; + const b = ids[j]; + if (a !== undefined && b !== undefined) union(a, b); + } + advisoryKeys.push(ids[0] as string); + } + + // Group advisories by cluster root, preserving the input order of + // first appearance so the eventual sort step gets a stable seed. + const clusters = new Map(); + for (let i = 0; i < advisories.length; i++) { + const advisory = advisories[i]; + if (!advisory) continue; + const key = advisoryKeys[i]; + if (key === undefined) continue; + const root = find(key); + const bucket = clusters.get(root); + if (bucket) bucket.push(advisory); + else clusters.set(root, [advisory]); + } + + const merged: VulnerabilityDetail[] = []; + for (const cluster of clusters.values()) { + merged.push(mergeAdvisoryCluster(cluster)); + } + return merged; +} + +function mergeAdvisoryCluster( + cluster: readonly VulnerabilityDetail[], +): VulnerabilityDetail { + if (cluster.length === 1) { + const only = cluster[0]; + if (!only) throw new Error("empty advisory cluster"); // unreachable + return only; + } + + const canonical = pickCanonical(cluster); + + // Union of all known identifiers across the cluster, with the + // canonical's own `osvId` removed from the alias list (it lives on + // `osvId` instead). + const aliasSet = new Set(); + for (const member of cluster) { + if (member.osvId) aliasSet.add(member.osvId); + for (const alias of member.aliases ?? []) { + if (alias) aliasSet.add(alias); + } + } + if (canonical.osvId) aliasSet.delete(canonical.osvId); + const aliases = Array.from(aliasSet).sort(); + + const affectedRangesSet = new Set(); + const fixedInSet = new Set(); + let isMalicious = false; + // Latest `modifiedAt` wins, but only counts entries that explicitly + // carry one — a sibling whose `modifiedAt` is absent should not + // promote its `publishedAt` into the merged record. + let latestModifiedAt: string | undefined = canonical.modifiedAt; + let withdrawnAt: string | undefined; + let allWithdrawn = true; + // Take the maximum severity score across the cluster. OSV and + // RUSTSEC occasionally disagree on the score for the same CVE; the + // safer choice for security tooling is the higher band, not the + // canonical's score (which may be lower because GHSA wins the + // canonical-pick on a prefix tiebreaker). + let maxSeverityScore = + typeof canonical.severityScore === "number" ? canonical.severityScore : 0; + + for (const member of cluster) { + for (const range of member.affectedVersionRanges ?? []) { + affectedRangesSet.add(range); + } + for (const fix of member.fixedInVersions ?? []) { + fixedInSet.add(fix); + } + if (member.isMalicious === true) isMalicious = true; + if (member.modifiedAt) { + if (!latestModifiedAt || member.modifiedAt > latestModifiedAt) { + latestModifiedAt = member.modifiedAt; + } + } + if (member.withdrawnAt) { + if (!withdrawnAt || member.withdrawnAt > withdrawnAt) { + withdrawnAt = member.withdrawnAt; + } + } else { + allWithdrawn = false; + } + // Withdrawn members don't contribute severity. The merged record + // surfaces the highest *currently authoritative* score; promoting + // a retracted advisory's score would mislead callers about an + // active vulnerability that no longer claims that band. + if ( + !member.withdrawnAt && + typeof member.severityScore === "number" && + member.severityScore > maxSeverityScore + ) { + maxSeverityScore = member.severityScore; + } + } + + const merged: VulnerabilityDetail = { + ...canonical, + aliases, + }; + if (maxSeverityScore > 0) { + merged.severityScore = maxSeverityScore; + } + if (affectedRangesSet.size > 0) { + merged.affectedVersionRanges = Array.from(affectedRangesSet); + } + if (fixedInSet.size > 0) { + merged.fixedInVersions = Array.from(fixedInSet); + } + if (isMalicious) merged.isMalicious = true; + // `buildAdvisory` later drops `modifiedAt` if it equals `publishedAt`, + // so we only need to ensure the field reflects the latest real + // modification timestamp here. The seed already filtered out the + // "no modifiedAt anywhere in the cluster" case. + if (latestModifiedAt) { + merged.modifiedAt = latestModifiedAt; + } else { + delete merged.modifiedAt; + } + // Only mark the merged advisory as withdrawn when every cluster + // member is withdrawn — a single non-withdrawn member means the + // vulnerability is still active under at least one source. + if (allWithdrawn && withdrawnAt) { + merged.withdrawnAt = withdrawnAt; + } else { + delete merged.withdrawnAt; + } + return merged; +} + +function pickCanonical( + cluster: readonly VulnerabilityDetail[], +): VulnerabilityDetail { + const ranked = cluster.slice().sort((a, b) => { + const aHasScore = + typeof a.severityScore === "number" && a.severityScore > 0 ? 1 : 0; + const bHasScore = + typeof b.severityScore === "number" && b.severityScore > 0 ? 1 : 0; + if (aHasScore !== bHasScore) return bHasScore - aHasScore; + + const aRank = idPrefixRank(a.osvId); + const bRank = idPrefixRank(b.osvId); + if (aRank !== bRank) return aRank - bRank; + + const aId = a.osvId ?? ""; + const bId = b.osvId ?? ""; + if (aId !== bId) return aId < bId ? -1 : 1; + return 0; + }); + const winner = ranked[0]; + if (!winner) throw new Error("empty cluster passed to pickCanonical"); // unreachable + return winner; +} + +/** + * Lower rank wins. GHSA carries severity reliably; RUSTSEC entries + * round-tripped through OSV often lose it. Other prefixes fall back + * to a generic bucket between the two known forms. A missing id is + * the worst rank — it cannot be cited and rarely surfaces in + * production (every backend-fed advisory carries an `osvId`). + */ +function idPrefixRank(id: string | undefined): number { + if (!id) return 99; // fallback bucket — all real ids outrank this + if (id.startsWith("GHSA-")) return 0; + if (id.startsWith("RUSTSEC-")) return 2; + return 1; +} + // -------------------------------------------------------------------- // Advisory shaping // -------------------------------------------------------------------- diff --git a/src/shared/unified-search-response.test.ts b/src/shared/unified-search-response.test.ts index b429f11f..fa16eb66 100644 --- a/src/shared/unified-search-response.test.ts +++ b/src/shared/unified-search-response.test.ts @@ -1,7 +1,11 @@ import { describe, expect, it } from "bun:test"; -import type { UnifiedSearchParams } from "../services/code-navigation-service.js"; +import type { + UnifiedSearchOutcome, + UnifiedSearchParams, +} from "../services/code-navigation-service.js"; import { defaultUnifiedSearchOutcome } from "../services/test-helpers.js"; import { + buildSourceStatusWarnings, buildUnifiedSearchErrorPayload, buildUnifiedSearchStatusPayload, buildUnifiedSearchSuccessPayload, @@ -132,6 +136,197 @@ describe("buildUnifiedSearchSuccessPayload", () => { }); }); +describe("buildSourceStatusWarnings — sourceStatus → warnings promotion", () => { + it("returns empty when source status is undefined or empty", () => { + expect(buildSourceStatusWarnings(undefined)).toEqual([]); + expect(buildSourceStatusWarnings([])).toEqual([]); + }); + + it("promotes incompatibleQueryFeatures into a structured message", () => { + const warnings = buildSourceStatusWarnings([ + { + source: "docs", + targetLabel: "npm:zod@4.3.6", + incompatibleQueryFeatures: ["kind"], + note: "Incompatible with query features: kind", + }, + ]); + expect(warnings).toEqual([ + "Source 'docs' for npm:zod@4.3.6: incompatible query features [kind]", + ]); + }); + + it("combines multiple reasons in a single warning", () => { + const warnings = buildSourceStatusWarnings([ + { + source: "docs", + targetLabel: "npm:express@5.2.1", + incompatibleQueryFeatures: ["lang"], + ignoredFilters: ["fileIntent"], + }, + ]); + expect(warnings.length).toBe(1); + expect(warnings[0]).toContain("incompatible query features [lang]"); + expect(warnings[0]).toContain("ignored filters [fileIntent]"); + }); + + it("falls back to the free-form note when no structured fields fired", () => { + const warnings = buildSourceStatusWarnings([ + { + source: "code", + targetLabel: "npm:express@5.2.1", + note: "Index rebuilt 5 minutes ago.", + }, + ]); + expect(warnings).toEqual([ + "Source 'code' for npm:express@5.2.1: Index rebuilt 5 minutes ago.", + ]); + }); + + it("produces one warning per source-status entry, preserving order", () => { + const warnings = buildSourceStatusWarnings([ + { + source: "docs", + targetLabel: "npm:zod@4.3.6", + incompatibleQueryFeatures: ["kind"], + }, + { + source: "code", + targetLabel: "npm:zod@4.3.6", + ignoredFilters: ["pathPrefix"], + }, + ]); + expect(warnings.length).toBe(2); + expect(warnings[0]).toContain("docs"); + expect(warnings[1]).toContain("code"); + }); +}); + +describe("buildUnifiedSearchSuccessPayload — sourceStatus warnings on completed payloads", () => { + function buildOutcomeWithStatus( + overrides: Record, + ): UnifiedSearchOutcome { + if (defaultUnifiedSearchOutcome.state !== "completed") { + throw new Error("expected completed fixture"); + } + const sourceStatus = [ + { + ...defaultUnifiedSearchOutcome.result.sourceStatus[0], + ...overrides, + }, + ]; + return { + ...defaultUnifiedSearchOutcome, + result: { + ...defaultUnifiedSearchOutcome.result, + sourceStatus, + }, + } as UnifiedSearchOutcome; + } + + const params: UnifiedSearchParams = { + targets: [{ registry: "NPM", packageName: "zod" }], + query: "parse kind:function", + sources: ["DOCS"], + limit: 20, + offset: 0, + waitTimeoutMs: 20_000, + }; + + it("emits warnings[] when a source reports incompatibleQueryFeatures (B5 repro)", () => { + const outcome = buildOutcomeWithStatus({ + source: "DOCS", + targetLabel: "npm:zod@4.3.6", + incompatibleQueryFeatures: ["kind"], + note: "Incompatible with query features: kind", + }); + const payload = buildUnifiedSearchSuccessPayload( + params, + "parse kind:function", + "parse kind:function", + outcome, + ); + expect(payload.completed).toBe(true); + expect(payload.warnings).toEqual([ + "Source 'docs' for npm:zod@4.3.6: incompatible query features [kind]", + ]); + // Structured detail is still available alongside. + expect(payload.sourceStatus?.[0]?.incompatibleQueryFeatures).toEqual([ + "kind", + ]); + }); + + it("omits warnings[] when sourceStatus is healthy", () => { + const payload = buildUnifiedSearchSuccessPayload( + params, + "router middleware", + "router middleware", + defaultUnifiedSearchOutcome, + ); + expect(payload.warnings).toBeUndefined(); + }); + + it("includes parser warnings ahead of sourceStatus warnings at top level", () => { + if (defaultUnifiedSearchOutcome.state !== "completed") { + throw new Error("expected completed fixture"); + } + const outcome = { + ...defaultUnifiedSearchOutcome, + result: { + ...defaultUnifiedSearchOutcome.result, + queryWarnings: ["unrecognised qualifier 'xyz:'"], + sourceStatus: [ + { + ...defaultUnifiedSearchOutcome.result.sourceStatus[0], + incompatibleQueryFeatures: ["kind"], + }, + ], + }, + } as UnifiedSearchOutcome; + const payload = buildUnifiedSearchSuccessPayload( + params, + "parse kind:function", + "parse kind:function", + outcome, + ); + expect(payload.warnings).toEqual([ + "unrecognised qualifier 'xyz:'", + expect.stringContaining("incompatible query features [kind]"), + ]); + // Parser warnings remain on the query echo for callers that + // specifically inspect the parser-warning surface. + expect(payload.query.warnings).toEqual(["unrecognised qualifier 'xyz:'"]); + }); +}); + +describe("buildUnifiedSearchStatusPayload — combined warnings", () => { + it("appends sourceStatus warnings after parser warnings", () => { + if (defaultUnifiedSearchOutcome.state !== "completed") { + throw new Error("expected completed fixture"); + } + const outcome = { + ...defaultUnifiedSearchOutcome, + result: { + ...defaultUnifiedSearchOutcome.result, + queryWarnings: ["unrecognised qualifier 'xyz:'"], + sourceStatus: [ + { + ...defaultUnifiedSearchOutcome.result.sourceStatus[0], + incompatibleQueryFeatures: ["kind"], + }, + ], + }, + } as UnifiedSearchOutcome; + + const payload = buildUnifiedSearchStatusPayload(outcome); + if (!payload.completed) throw new Error("expected completed payload"); + expect(payload.result.warnings).toEqual([ + "unrecognised qualifier 'xyz:'", + expect.stringContaining("incompatible query features [kind]"), + ]); + }); +}); + describe("buildUnifiedSearchErrorPayload", () => { it("maps errors to the shared error envelope", () => { const payload = buildUnifiedSearchErrorPayload(new Error("boom")); diff --git a/src/shared/unified-search-response.ts b/src/shared/unified-search-response.ts index be0fe4b4..db8ad07e 100644 --- a/src/shared/unified-search-response.ts +++ b/src/shared/unified-search-response.ts @@ -97,6 +97,14 @@ export interface UnifiedSearchCompletedPayload { nextOffset?: number; results: UnifiedSearchHitPayload[]; searchRef?: string; + /** + * Top-level execution warnings derived from `sourceStatus`. Promoted + * here so agents see them without inspecting the nested + * `sourceStatus` block — the structured detail still lives there for + * callers that need it. Parser-level warnings stay in + * {@link UnifiedSearchQueryEcho.warnings}. + */ + warnings?: string[]; sourceStatus?: UnifiedSearchSourceStatusPayload[]; } @@ -108,6 +116,7 @@ export interface UnifiedSearchIncompletePayload { results: UnifiedSearchHitPayload[]; searchRef: string; progress?: UnifiedSearchProgressPayload; + warnings?: string[]; sourceStatus?: UnifiedSearchSourceStatusPayload[]; } @@ -170,6 +179,8 @@ export function buildUnifiedSearchSuccessPayload( if (progress) payload.progress = progress; const sourceStatus = compactSourceStatus(result?.sourceStatus); if (sourceStatus) payload.sourceStatus = sourceStatus; + const combinedWarnings = combineWarnings(warnings, sourceStatus); + if (combinedWarnings.length > 0) payload.warnings = combinedWarnings; return payload; } @@ -186,9 +197,33 @@ export function buildUnifiedSearchSuccessPayload( if (outcome.searchRef) completed.searchRef = outcome.searchRef; const sourceStatus = compactSourceStatus(outcome.result.sourceStatus); if (sourceStatus) completed.sourceStatus = sourceStatus; + const combinedWarnings = combineWarnings(warnings, sourceStatus); + if (combinedWarnings.length > 0) completed.warnings = combinedWarnings; return completed; } +/** + * Compose the top-level `warnings[]` array from parser warnings + * (compile-time issues with the query string) and sourceStatus-derived + * warnings (runtime issues affecting result completeness). Parser + * warnings come first so query-shape issues remain at the head of the + * list; the same ordering applies in `buildUnifiedSearchStatusResultPayload`. + * + * `query.warnings` on the echoed query stays populated for callers that + * specifically inspect the parser-warning surface; including parser + * warnings here too gives text-v1 readers a single, consistent + * `warnings:` preamble. + */ +function combineWarnings( + parserWarnings: readonly string[], + sourceStatus: UnifiedSearchSourceStatusPayload[] | undefined, +): string[] { + const out: string[] = []; + if (parserWarnings.length > 0) out.push(...parserWarnings); + out.push(...buildSourceStatusWarnings(sourceStatus)); + return out; +} + export function buildUnifiedSearchErrorPayload( error: unknown, ): UnifiedSearchErrorPayload { @@ -240,14 +275,15 @@ function buildUnifiedSearchStatusResultPayload( if (result.page.hasMore) { payload.nextOffset = result.page.offset + result.page.returned; } - if (result.queryWarnings.length > 0) { - payload.warnings = result.queryWarnings; - } if (result.sources.length > 0) { payload.sources = result.sources.map((entry) => entry.toLowerCase()); } const sourceStatus = compactSourceStatus(result.sourceStatus); if (sourceStatus) payload.sourceStatus = sourceStatus; + const combinedWarnings = combineWarnings(result.queryWarnings, sourceStatus); + if (combinedWarnings.length > 0) { + payload.warnings = combinedWarnings; + } return payload; } @@ -372,6 +408,76 @@ function compactProgress( return payload; } +/** + * Promote noteworthy `sourceStatus` entries into top-level human-readable + * warning strings so callers see them without inspecting the nested + * structured block. Mitigation for backend issue B5 (search responses + * with `sources: ["docs"]` plus a `kind:`/`lang:` qualifier silently + * return empty results because the only signal — incompatibility — is + * buried inside `sourceStatus[].note`). + * + * Order: each compacted entry contributes at most one warning. We + * derive the message from the structured fields first; if none of + * those fired but a free-form `note` is present, the note alone is + * promoted. Iteration order matches `compactSourceStatus`, which is + * the backend's order. + */ +export function buildSourceStatusWarnings( + sourceStatus: UnifiedSearchSourceStatusPayload[] | undefined, +): string[] { + if (!sourceStatus || sourceStatus.length === 0) return []; + const warnings: string[] = []; + for (const entry of sourceStatus) { + const message = warningForEntry(entry); + if (message !== undefined) warnings.push(message); + } + return warnings; +} + +function warningForEntry( + entry: UnifiedSearchSourceStatusPayload, +): string | undefined { + const reasons: string[] = []; + if (entry.incompatibleQueryFeatures?.length) { + reasons.push( + `incompatible query features [${entry.incompatibleQueryFeatures.join(", ")}]`, + ); + } + if (entry.ignoredQueryFeatures?.length) { + reasons.push( + `ignored query features [${entry.ignoredQueryFeatures.join(", ")}]`, + ); + } + if (entry.incompatibleFilters?.length) { + reasons.push( + `incompatible filters [${entry.incompatibleFilters.join(", ")}]`, + ); + } + if (entry.ignoredFilters?.length) { + reasons.push(`ignored filters [${entry.ignoredFilters.join(", ")}]`); + } + // Healthy lifecycle states (`INDEXED`, `CURRENT`, `STALE`) are + // already filtered out upstream in `compactSourceStatusEntry`; if + // these fields are present here, the state is genuinely worth + // surfacing. + if (entry.indexingStatus) { + reasons.push(`indexing status ${entry.indexingStatus}`); + } + if (entry.codeIndexState) { + reasons.push(`code index state ${entry.codeIndexState}`); + } + // Source/target prefix anchors the message so an agent reading + // multi-source warnings can tell which target each refers to. + const prefix = `Source '${entry.source}' for ${entry.targetLabel}`; + if (reasons.length > 0) { + return `${prefix}: ${reasons.join("; ")}`; + } + if (entry.note) { + return `${prefix}: ${entry.note}`; + } + return undefined; +} + function compactSourceStatus( sourceStatus: UnifiedSearchSourceStatus[] | undefined, ): UnifiedSearchSourceStatusPayload[] | undefined { diff --git a/src/shared/unified-search-text.test.ts b/src/shared/unified-search-text.test.ts index a47fb68e..b50e65f4 100644 --- a/src/shared/unified-search-text.test.ts +++ b/src/shared/unified-search-text.test.ts @@ -196,6 +196,34 @@ describe("renderUnifiedSearchSuccess", () => { expect(text).toContain("ignored=fileIntent"); }); + it("renders a warnings preamble when payload-level warnings are populated", () => { + const text = renderUnifiedSearchSuccess( + completed([], { + warnings: [ + "Source 'docs' for npm:zod@4.3.6: incompatible query features [kind]", + ], + sourceStatus: [ + { + source: "docs", + targetLabel: "npm:zod@4.3.6", + incompatibleQueryFeatures: ["kind"], + }, + ], + }), + ); + expect(text).toContain("warnings:"); + expect(text).toContain( + " - Source 'docs' for npm:zod@4.3.6: incompatible query features [kind]", + ); + // Source notes block still rendered for structured detail. + expect(text).toContain("source notes:"); + }); + + it("omits the warnings preamble when no warnings are present", () => { + const text = renderUnifiedSearchSuccess(completed([codeHit()])); + expect(text).not.toContain("warnings:"); + }); + it("separates multiple hits with a blank line", () => { const text = renderUnifiedSearchSuccess( completed([codeHit(), docsHit(), symbolHit()]), diff --git a/src/shared/unified-search-text.ts b/src/shared/unified-search-text.ts index 1618b56e..ac3c4850 100644 --- a/src/shared/unified-search-text.ts +++ b/src/shared/unified-search-text.ts @@ -167,6 +167,13 @@ function formatScore(score: number): string { function buildTrailer(payload: SearchSuccessPayload): string[] { const lines: string[] = []; + if (payload.warnings && payload.warnings.length > 0) { + lines.push("warnings:"); + for (const warning of payload.warnings) { + lines.push(` - ${warning}`); + } + } + if (payload.hasMore) { const nextOffsetHint = typeof payload.nextOffset === "number"