diff --git a/app/src/__tests__/e2e/logs/logs-mobile-overflow.spec.ts b/app/src/__tests__/e2e/logs/logs-mobile-overflow.spec.ts index e2271922..9d072ac1 100644 --- a/app/src/__tests__/e2e/logs/logs-mobile-overflow.spec.ts +++ b/app/src/__tests__/e2e/logs/logs-mobile-overflow.spec.ts @@ -70,4 +70,189 @@ test.describe('Logs mobile layout', () => { }); expect(hasHorizontalScrollbar).toBeFalsy(); }); + + test('whole-card expansion: every row expands, quick-rule is excluded, no overflow with long labels', async ({ page }) => { + await registerMocks(page, { + authenticated: true, + customProfiles: [{ id: 'prof1', profile_id: 'prof1', name: 'Default', settings: { logs: { enabled: true } } }] + }); + + // Register the logs route AFTER registerMocks so it is tested BEFORE the catch-all + // route (Playwright matches routes in reverse registration order). The catch-all in + // registerMocks matches `/api/v1/profiles` and would otherwise shadow this endpoint, + // returning the profiles array instead of our logs payload. + const now = new Date().toISOString(); + const items = [ + // Blocked row WITH reasons — deliberately long ids to challenge layout / overflow + { + profile_id: 'prof1', + timestamp: now, + status: 'blocked', + protocol: 'dns', + device_id: 'device-with-reasons', + client_ip: '10.0.0.1', + dns_request: { domain: 'blocked-with-reasons.example-longdomainforlayout-validation.test' }, + reasons: [ + 'blocklist: very-long-blocklist-identifier-xxxxxxxxxxxxxxxxxxxx', + 'service: another-long-service-id-yyyyyyyyyyyyyyyyyyyy' + ] + }, + // Processed row WITHOUT reasons — now also expandable (detail grid, no reasons block) + { + profile_id: 'prof1', + timestamp: now, + status: 'processed', + protocol: 'dns', + device_id: 'device-no-reasons', + client_ip: '10.0.0.2', + dns_request: { domain: 'processed-no-reasons.example.test' } + } + ]; + await page.route(/\/api\/v1\/profiles\/prof1\/logs/i, route => { + route.fulfill({ status: 200, contentType: 'application/json', body: JSON.stringify(items) }); + }); + + await page.goto('/query-logs'); + + const scrollContainer = page.getByTestId('logs-scroll-container'); + await scrollContainer.first().waitFor({ state: 'attached', timeout: 10000 }); + + // Every row is expandable now → one toggle + one panel per mocked row (2). + const toggles = page.getByTestId('querylog-card-toggle'); + await expect(toggles).toHaveCount(items.length); + const panels = page.getByTestId('querylog-expanded-panel'); + await expect(panels).toHaveCount(items.length); + + const firstPanel = panels.nth(0); + const secondPanel = panels.nth(1); + + // Collapsed initial state + await expect(toggles.nth(0)).toHaveAttribute('aria-expanded', 'false'); + await expect(firstPanel).toHaveAttribute('data-expanded', 'false'); + + // Quick-rule button is excluded from the overlay: clicking it must NOT expand the card. + await page.getByTestId('logs-quick-rule-button').nth(0).click(); + await expect(firstPanel).toHaveAttribute('data-expanded', 'false'); + // Close any sheet the quick-rule action opened so it doesn't cover the cards below. + await page.keyboard.press('Escape'); + + // Keyboard: focus the first card's toggle and press Enter to expand. + await toggles.nth(0).focus(); + await page.keyboard.press('Enter'); + await expect(toggles.nth(0)).toHaveAttribute('aria-expanded', 'true'); + await expect(firstPanel).toHaveAttribute('data-expanded', 'true'); + + // Expanded panel shows the detail grid (protocol + timestamp always render) and the reasons block. + await expect(firstPanel.getByTestId('querylog-detail-grid')).toBeVisible(); + await expect(firstPanel.getByTestId('querylog-detail-protocol')).toBeVisible(); + await expect(firstPanel.getByTestId('querylog-detail-timestamp')).toBeVisible(); + await expect(firstPanel.getByTestId('querylog-reasons')).toBeVisible(); + + // The processed (no-reasons) row expands too: detail grid visible, but no reasons block. + await toggles.nth(1).click(); + await expect(secondPanel).toHaveAttribute('data-expanded', 'true'); + await expect(secondPanel.getByTestId('querylog-detail-grid')).toBeVisible(); + await expect(secondPanel.getByTestId('querylog-reasons')).toHaveCount(0); + + // Re-run overflow assertions AFTER expansion, with the long labels rendered + const result = await page.evaluate(() => { + const docEl = document.documentElement; + const body = document.body; + const vw = window.innerWidth; + const sc = document.querySelector('[data-testid="logs-scroll-container"]') as HTMLElement | null; + const scrollWidthDoc = Math.max( + body.scrollWidth, + docEl.scrollWidth, + body.offsetWidth, + docEl.offsetWidth + ); + const scrollingElWidth = document.scrollingElement ? document.scrollingElement.scrollWidth : docEl.scrollWidth; + const scOverflow = sc ? sc.scrollWidth - sc.clientWidth : 0; + return { vw, scrollWidthDoc, scrollingElWidth, scOverflow }; + }); + expect(result.scrollingElWidth).toBeLessThanOrEqual(result.vw + 1); + expect(result.scrollWidthDoc).toBeLessThanOrEqual(result.vw + 1); + expect(result.scOverflow).toBeLessThanOrEqual(1); + + // The expanded panel's bounding box must sit within the viewport horizontally + const viewport = page.viewportSize(); + const viewportWidth = viewport?.width ?? result.vw; + const box = await firstPanel.boundingBox(); + expect(box, 'Expected expanded panel to have a bounding box').not.toBeNull(); + if (box) { + expect(box.x).toBeGreaterThanOrEqual(0); + expect(box.x + box.width).toBeLessThanOrEqual(viewportWidth + 1); + } + }); + + test('expanded card collapses when clicking anywhere, including the expanded panel', async ({ page }) => { + await registerMocks(page, { + authenticated: true, + customProfiles: [{ id: 'prof1', profile_id: 'prof1', name: 'Default', settings: { logs: { enabled: true } } }] + }); + const now = new Date().toISOString(); + const items = [ + { + profile_id: 'prof1', timestamp: now, status: 'blocked', protocol: 'dns', + device_id: 'd1', client_ip: '10.0.0.1', + dns_request: { domain: 'blocked.example.test' }, + reasons: ['blocklist: some-blocklist-id'] + } + ]; + await page.route(/\/api\/v1\/profiles\/prof1\/logs/i, route => { + route.fulfill({ status: 200, contentType: 'application/json', body: JSON.stringify(items) }); + }); + + await page.goto('/query-logs'); + await page.getByTestId('logs-scroll-container').first().waitFor({ state: 'attached', timeout: 10000 }); + + const toggle = page.getByTestId('querylog-card-toggle').first(); + const panel = page.getByTestId('querylog-expanded-panel').first(); + + // Expand. + await toggle.click(); + await expect(panel).toHaveAttribute('data-expanded', 'true'); + + // Click inside the EXPANDED PANEL region (below the header) — must collapse the card. + const box = await panel.boundingBox(); + expect(box, 'Expected an expanded panel bounding box').not.toBeNull(); + if (box) { + await page.mouse.click(box.x + box.width / 2, box.y + box.height / 2); + } + await expect(panel).toHaveAttribute('data-expanded', 'false'); + }); + + test('mobile: one-time expand hint shows, dismisses after first expand, and stays gone', async ({ page }, testInfo) => { + // The hint is mobile-only (md:hidden); skip on desktop projects. + test.skip(!/(chromium-mobile|iphone15pro)/i.test(testInfo.project.name), 'mobile-only hint'); + + await registerMocks(page, { + authenticated: true, + customProfiles: [{ id: 'prof1', profile_id: 'prof1', name: 'Default', settings: { logs: { enabled: true } } }] + }); + const now = new Date().toISOString(); + const items = [ + { profile_id: 'prof1', timestamp: now, status: 'processed', protocol: 'dns', device_id: 'd1', client_ip: '10.0.0.1', dns_request: { domain: 'a.example.test' } }, + { profile_id: 'prof1', timestamp: now, status: 'blocked', protocol: 'dns', device_id: 'd2', client_ip: '10.0.0.2', dns_request: { domain: 'b.example.test' } } + ]; + await page.route(/\/api\/v1\/profiles\/prof1\/logs/i, route => { + route.fulfill({ status: 200, contentType: 'application/json', body: JSON.stringify(items) }); + }); + + await page.goto('/query-logs'); + await page.getByTestId('logs-scroll-container').first().waitFor({ state: 'attached', timeout: 10000 }); + + // Hint is visible on first visit. + const hint = page.getByTestId('logs-expand-hint'); + await expect(hint).toBeVisible(); + + // Expanding a row dismisses the hint. + await page.getByTestId('querylog-card-toggle').first().click(); + await expect(hint).toHaveCount(0); + + // Persisted: reload keeps it gone. + await page.reload(); + await page.getByTestId('logs-scroll-container').first().waitFor({ state: 'attached', timeout: 10000 }); + await expect(page.getByTestId('logs-expand-hint')).toHaveCount(0); + }); }); diff --git a/app/src/__tests__/unit/QueryLogCard.test.tsx b/app/src/__tests__/unit/QueryLogCard.test.tsx index 7fd98c56..1849a78d 100644 --- a/app/src/__tests__/unit/QueryLogCard.test.tsx +++ b/app/src/__tests__/unit/QueryLogCard.test.tsx @@ -25,7 +25,7 @@ function stubDesktopMatchMedia(isDesktop: boolean) { }; } -describe('QueryLogCard truncation interactions', () => { +describe('QueryLogCard truncation display', () => { beforeEach(() => { // Reset viewport width // Override viewport width for desktop simulation @@ -49,9 +49,6 @@ describe('QueryLogCard truncation interactions', () => { expect(fullEl).toHaveTextContent(deviceId); expect(fullEl.textContent).toHaveLength(deviceId.length); expect(fullEl.textContent?.endsWith('…')).toBeFalsy(); - // Tooltip still present wrapping element; hover should not change content - fireEvent.mouseEnter(fullEl); - expect(fullEl).toHaveTextContent(deviceId); }); test('desktop domain display strips trailing dot', () => { @@ -71,7 +68,7 @@ describe('QueryLogCard truncation interactions', () => { expect(domainSpan).not.toHaveTextContent(/\.$/); }); - test('mobile tap expands truncated domain (threshold 65)', () => { + test('mobile renders a static truncated domain span (no tap-to-reveal)', () => { stubDesktopMatchMedia(false); // Override viewport width for mobile simulation (window as unknown as { innerWidth: number }).innerWidth = 375; @@ -87,13 +84,157 @@ describe('QueryLogCard truncation interactions', () => { dns_request: { domain: longDomain } }; render(); - const truncatedDomainBtn = screen.getByTestId('querylog-domain-truncated'); - expect(truncatedDomainBtn).toBeInTheDocument(); - // Verify it contains ellipsis at end - expect(truncatedDomainBtn.textContent).toMatch(/…$/); - fireEvent.click(truncatedDomainBtn); - const fullDomainSpan = screen.getByTestId('querylog-domain-full'); - expect(fullDomainSpan).toHaveTextContent(longDomain); + const truncatedDomain = screen.getByTestId('querylog-domain-truncated'); + expect(truncatedDomain).toBeInTheDocument(); + // Static truncated text ends with an ellipsis; it is a plain span (not a button). + expect(truncatedDomain.textContent).toMatch(/…$/); + expect(truncatedDomain.tagName).toBe('SPAN'); + }); +}); + +describe('QueryLogCard whole-card expansion', () => { + beforeEach(() => { + (window as unknown as { innerWidth: number }).innerWidth = 1440; + stubDesktopMatchMedia(true); + }); + + const baseLog: ModelQueryLog = { + profile_id: 'p-exp', + timestamp: '2026-06-15T10:20:30.000Z', + status: 'processed', + protocol: 'dns', + device_id: 'expand-device', + client_ip: '10.0.0.9', + dns_request: { domain: 'expand.example.com', query_type: 'A', response_code: 'NOERROR', dnssec: true } + }; + + test('renders the whole-card toggle', () => { + render(); + expect(screen.getByTestId('querylog-card-toggle')).toBeInTheDocument(); + }); + + test('clicking the toggle flips the expanded panel state', () => { + render(); + const toggle = screen.getByTestId('querylog-card-toggle'); + const panel = screen.getByTestId('querylog-expanded-panel'); + expect(panel).toHaveAttribute('data-expanded', 'false'); + expect(toggle).toHaveAttribute('aria-expanded', 'false'); + fireEvent.click(toggle); + expect(panel).toHaveAttribute('data-expanded', 'true'); + expect(toggle).toHaveAttribute('aria-expanded', 'true'); + }); + + test('expanded panel shows the detail grid with protocol and timestamp', () => { + render(); + fireEvent.click(screen.getByTestId('querylog-card-toggle')); + expect(screen.getByTestId('querylog-detail-grid')).toBeInTheDocument(); + expect(screen.getByTestId('querylog-detail-protocol')).toHaveTextContent('DNS'); + expect(screen.getByTestId('querylog-detail-timestamp')).toBeInTheDocument(); + }); + + test('row with reasons renders the reasons block', () => { + const log: ModelQueryLog = { + ...baseLog, + status: 'blocked', + reasons: ['blocklist: some-blocklist-id'] + }; + render(); + fireEvent.click(screen.getByTestId('querylog-card-toggle')); + expect(screen.getByTestId('querylog-reasons')).toBeInTheDocument(); + }); + + test('row without reasons omits the reasons block but still expands', () => { + render(); + fireEvent.click(screen.getByTestId('querylog-card-toggle')); + expect(screen.getByTestId('querylog-detail-grid')).toBeInTheDocument(); + expect(screen.queryByTestId('querylog-reasons')).not.toBeInTheDocument(); + }); + + test('domain-logging-disabled row is still expandable and shows a placeholder', () => { + const log: ModelQueryLog = { + ...baseLog, + dns_request: undefined as unknown as ModelQueryLog['dns_request'] + }; + render(); + fireEvent.click(screen.getByTestId('querylog-card-toggle')); + expect(screen.getByTestId('querylog-detail-domain')).toHaveTextContent('Domain logging disabled'); + }); + + test('there is no visible chevron indicator', () => { + render(); + expect(screen.queryByTestId('querylog-expand-indicator')).not.toBeInTheDocument(); + }); + + test('onExpand fires only when expanding (not when collapsing)', () => { + const onExpand = vi.fn(); + render(); + const toggle = screen.getByTestId('querylog-card-toggle'); + fireEvent.click(toggle); // expand + expect(onExpand).toHaveBeenCalledTimes(1); + fireEvent.click(toggle); // collapse + expect(onExpand).toHaveBeenCalledTimes(1); + }); + + test('shows the DNSSEC badge on the collapsed row when validated', () => { + render(); // baseLog has dns_request.dnssec === true + expect(screen.getByTestId('querylog-dnssec-badge')).toHaveTextContent('DNSSEC'); + }); + + test('omits the DNSSEC badge when neither validated nor failed', () => { + const log: ModelQueryLog = { + ...baseLog, + dns_request: { ...baseLog.dns_request, dnssec: false } + }; + render(); + expect(screen.queryByTestId('querylog-dnssec-badge')).not.toBeInTheDocument(); + }); + + test('shows a red (failed) DNSSEC badge when validation failed', () => { + const log: ModelQueryLog = { + ...baseLog, + status: 'processed', + dns_request: { ...baseLog.dns_request, dnssec: false, response_code: 'SERVFAIL' }, + reasons: ['dnssec_failed'], + }; + render(); + const badge = screen.getByTestId('querylog-dnssec-badge'); + expect(badge).toHaveTextContent('DNSSEC'); + expect(badge).toHaveAttribute('data-dnssec', 'failed'); + }); + + test('labels the reason "Block reason" for a DNSSEC-failed row (not "Allow reason")', () => { + const log: ModelQueryLog = { + ...baseLog, + status: 'processed', + dns_request: { ...baseLog.dns_request, dnssec: false, response_code: 'SERVFAIL' }, + reasons: ['dnssec_failed'], + }; + render(); + fireEvent.click(screen.getByTestId('querylog-card-toggle')); + const reasons = screen.getByTestId('querylog-reasons'); + expect(reasons).toHaveTextContent('Block reason'); + expect(reasons).not.toHaveTextContent('Allow reason'); + }); + + test('DNSSEC detail field distinguishes the three states', () => { + const detailText = (log: ModelQueryLog) => { + const { unmount } = render(); + fireEvent.click(screen.getByTestId('querylog-card-toggle')); + const text = screen.getByTestId('querylog-detail-dnssec').textContent; + unmount(); + return text; + }; + // validated + expect(detailText(baseLog)).toBe('Validated'); + // unsigned (dnssec false, no failure reason) + expect(detailText({ ...baseLog, dns_request: { ...baseLog.dns_request, dnssec: false } })).toBe('No DNSSEC'); + // failed (bogus) + expect(detailText({ + ...baseLog, + status: 'processed', + dns_request: { ...baseLog.dns_request, dnssec: false, response_code: 'SERVFAIL' }, + reasons: ['dnssec_failed'], + })).toBe('Validation failed'); }); }); @@ -141,4 +282,3 @@ describe('QueryLogCard quick rule button', () => { expect(onQuickRule).not.toHaveBeenCalled(); }); }); - diff --git a/app/src/__tests__/unit/ReasonBadges.test.tsx b/app/src/__tests__/unit/ReasonBadges.test.tsx new file mode 100644 index 00000000..d0ed0f5d --- /dev/null +++ b/app/src/__tests__/unit/ReasonBadges.test.tsx @@ -0,0 +1,65 @@ +import { render, screen } from '@testing-library/react'; +import '@testing-library/jest-dom'; +import { describe, test, expect } from 'vitest'; +import { ReasonBadges } from '@/components/ui/ReasonBadges'; + +const blocklistNames = { 'hagezi-tif': 'HaGeZi TIF', x: 'Blocklist X' }; +const serviceNames = { tiktok: 'TikTok' }; + +describe('ReasonBadges', () => { + test('renders resolved blocklist and service names', () => { + render( + + ); + const badges = screen.getAllByTestId('querylog-reason-badge'); + expect(badges).toHaveLength(2); + expect(badges[0]).toHaveTextContent('Blocklist: HaGeZi TIF'); + expect(badges[1]).toHaveTextContent('Service: TikTok'); + }); + + test('falls back to the raw id when the name map has no entry', () => { + render(); + expect(screen.getByTestId('querylog-reason-badge')).toHaveTextContent('Blocklist: unknown-id'); + }); + + test('renders nothing when there are no reasons', () => { + const { container } = render(); + expect(container).toBeEmptyDOMElement(); + expect(screen.queryByTestId('querylog-reason-badge')).not.toBeInTheDocument(); + }); + + test('collapses more than three chips into a +N overflow chip', () => { + render( + + ); + // 4 formatted chips → 3 visible + 1 overflow chip + expect(screen.getAllByTestId('querylog-reason-badge')).toHaveLength(3); + const overflow = screen.getByTestId('querylog-reason-badge-overflow'); + expect(overflow).toHaveTextContent('+1'); + }); + + test('does not render an overflow chip when there are three or fewer chips', () => { + render( + + ); + expect(screen.getAllByTestId('querylog-reason-badge')).toHaveLength(3); + expect(screen.queryByTestId('querylog-reason-badge-overflow')).not.toBeInTheDocument(); + }); +}); diff --git a/app/src/__tests__/unit/lib/formatReasons.test.ts b/app/src/__tests__/unit/lib/formatReasons.test.ts new file mode 100644 index 00000000..9503fddf --- /dev/null +++ b/app/src/__tests__/unit/lib/formatReasons.test.ts @@ -0,0 +1,111 @@ +import { describe, it, expect } from 'vitest'; +import { formatReasons } from '@/lib/formatReasons'; + +const blocklistNames = { 'hagezi-tif': 'HaGeZi TIF', 'x': 'Blocklist X' }; +const serviceNames = { 'tiktok': 'TikTok', 'y': 'Service Y' }; + +describe('formatReasons', () => { + it('maps a specific blocklist id to a resolved name', () => { + // tableRef: logs-reason-display-behaviour #1 + expect(formatReasons(['blocklist: hagezi-tif'], blocklistNames, serviceNames)).toEqual([ + { kind: 'blocklist', label: 'Blocklist: HaGeZi TIF' }, + ]); + }); + + it('renders a generic Blocklist chip when only the generic token is present', () => { + // tableRef: logs-reason-display-behaviour #2 + expect(formatReasons(['blocklists'], blocklistNames, serviceNames)).toEqual([ + { kind: 'blocklist', label: 'Blocklist' }, + ]); + }); + + it('collapses generic + specific blocklist into the specific chip', () => { + // tableRef: logs-reason-display-behaviour #3 + expect(formatReasons(['blocklists', 'blocklist: x'], blocklistNames, serviceNames)).toEqual([ + { kind: 'blocklist', label: 'Blocklist: Blocklist X' }, + ]); + }); + + it('folds the subdomain rule into the blocklist chip as a qualifier', () => { + // tableRef: logs-reason-display-behaviour #4 + expect( + formatReasons(['blocklist: x', 'blocklists_subdomains_rule'], blocklistNames, serviceNames) + ).toEqual([{ kind: 'blocklist', label: 'Blocklist: Blocklist X (subdomain)' }]); + }); + + it('maps a specific service id to a resolved name', () => { + // tableRef: logs-reason-display-behaviour #5 + expect(formatReasons(['service: tiktok'], blocklistNames, serviceNames)).toEqual([ + { kind: 'service', label: 'Service: TikTok' }, + ]); + }); + + it('renders a generic Service chip when only the generic token is present', () => { + // tableRef: logs-reason-display-behaviour #6 + expect(formatReasons(['services'], blocklistNames, serviceNames)).toEqual([ + { kind: 'service', label: 'Service' }, + ]); + }); + + it('collapses generic + specific service into the specific chip', () => { + // tableRef: logs-reason-display-behaviour #7 + expect(formatReasons(['services', 'service: y'], blocklistNames, serviceNames)).toEqual([ + { kind: 'service', label: 'Service: Service Y' }, + ]); + }); + + it('maps custom_rules to a Custom rule chip', () => { + // tableRef: logs-reason-display-behaviour #8 + expect(formatReasons(['custom_rules'], blocklistNames, serviceNames)).toEqual([ + { kind: 'custom_rule', label: 'Custom rule' }, + ]); + }); + + it('maps default_rule to a Default rule chip', () => { + // tableRef: logs-reason-display-behaviour #9 + expect(formatReasons(['default_rule'], blocklistNames, serviceNames)).toEqual([ + { kind: 'default', label: 'Default rule' }, + ]); + }); + + it('renders nothing for empty input', () => { + // tableRef: logs-reason-display-behaviour #10 + expect(formatReasons([], blocklistNames, serviceNames)).toEqual([]); + }); + + it('renders multiple same-tier chips in a stable order (blocklist then service)', () => { + // tableRef: logs-reason-display-behaviour #11 + expect( + formatReasons(['service: y', 'blocklist: x'], blocklistNames, serviceNames) + ).toEqual([ + { kind: 'blocklist', label: 'Blocklist: Blocklist X' }, + { kind: 'service', label: 'Service: Service Y' }, + ]); + }); + + it('falls back to the raw id when the name map has no entry', () => { + // tableRef: logs-reason-display-behaviour #12 + expect(formatReasons(['blocklist: unknown-id'], blocklistNames, serviceNames)).toEqual([ + { kind: 'blocklist', label: 'Blocklist: unknown-id' }, + ]); + }); + + it('works without name maps, falling back to raw ids', () => { + // tableRef: logs-reason-display-behaviour #12 + expect(formatReasons(['service: some-svc'])).toEqual([ + { kind: 'service', label: 'Service: some-svc' }, + ]); + }); + + it('maps dnssec_failed to a "DNSSEC validation failed" chip, shown first', () => { + // tableRef: logs-reason-display-behaviour #13 + expect(formatReasons(['dnssec_failed'])).toEqual([ + { kind: 'dnssec', label: 'DNSSEC validation failed' }, + ]); + // when combined with other reasons it is ordered first + expect(formatReasons(['default_rule', 'dnssec_failed'])).toEqual([ + { kind: 'dnssec', label: 'DNSSEC validation failed' }, + { kind: 'default', label: 'Default rule' }, + ]); + }); +}); diff --git a/app/src/components/ui/ReasonBadges.tsx b/app/src/components/ui/ReasonBadges.tsx new file mode 100644 index 00000000..504dba79 --- /dev/null +++ b/app/src/components/ui/ReasonBadges.tsx @@ -0,0 +1,58 @@ +import * as React from "react"; +import { Badge } from "@/components/ui/badge"; +import { Tooltip } from "@/components/ui/tooltip"; +import { cn } from "@/lib/utils"; +import { formatReasons } from "@/lib/formatReasons"; + +interface ReasonBadgesProps { + reasons: string[]; + blocklistNames?: Record; + serviceNames?: Record; + className?: string; +} + +// Show at most this many chips inline; the remainder collapse into a "+N" chip. +const MAX_VISIBLE = 3; + +/** + * Render query-log reason tokens as human-readable chips. + * + * Mapping is delegated to `formatReasons` (see + * docs/specs/logs-reason-display-behaviour.md). Overflow beyond MAX_VISIBLE + * chips collapses into a tooltip-backed "+N" chip. + */ +export function ReasonBadges({ reasons, blocklistNames, serviceNames, className }: ReasonBadgesProps) { + const formatted = formatReasons(reasons, blocklistNames, serviceNames); + if (formatted.length === 0) return null; + + const visible = formatted.slice(0, MAX_VISIBLE); + const overflow = formatted.slice(MAX_VISIBLE); + + return ( +
+ {visible.map((reason, i) => ( + + {reason.label} + + ))} + {overflow.length > 0 && ( + r.label).join(", ")}> + + +{overflow.length} + + + )} +
+ ); +} + +export default ReasonBadges; diff --git a/app/src/lib/formatReasons.ts b/app/src/lib/formatReasons.ts new file mode 100644 index 00000000..8eb1154a --- /dev/null +++ b/app/src/lib/formatReasons.ts @@ -0,0 +1,115 @@ +// formatReasons — map raw proxy reason tokens to human-readable chips. +// +// Source of truth: docs/specs/logs-reason-display-behaviour.md +// If the chip mapping changes, update that spec and formatReasons.test.ts with +// matching `tableRef: logs-reason-display-behaviour #N` annotations. + +export type ReasonKind = 'blocklist' | 'service' | 'custom_rule' | 'default' | 'subdomain' | 'dnssec'; + +export interface FormattedReason { + kind: ReasonKind; + label: string; +} + +const BLOCKLIST_PREFIX = 'blocklist: '; +const SERVICE_PREFIX = 'service: '; + +/** + * Convert stored proxy reason tokens into de-duplicated, ordered display chips. + * + * @param reasons Raw tokens from `ModelQueryLog.reasons` (order not assumed). + * @param blocklistNames Optional id → display-name map for `blocklist: `. + * @param serviceNames Optional id → display-name map for `service: `. + */ +export function formatReasons( + reasons: string[], + blocklistNames?: Record, + serviceNames?: Record, +): FormattedReason[] { + if (!reasons || reasons.length === 0) return []; + + // First-seen order preserved; a Set guards against duplicate ids. + const blocklistIds: string[] = []; + const blocklistIdSet = new Set(); + const serviceIds: string[] = []; + const serviceIdSet = new Set(); + let hasGenericBlocklist = false; + let hasGenericService = false; + let hasSubdomain = false; + let hasCustomRule = false; + let hasDefault = false; + let hasDnssecFailed = false; + + for (const reason of reasons) { + if (reason === 'dnssec_failed') { + hasDnssecFailed = true; + continue; + } + if (reason.startsWith(BLOCKLIST_PREFIX)) { + const id = reason.slice(BLOCKLIST_PREFIX.length); + if (!blocklistIdSet.has(id)) { + blocklistIdSet.add(id); + blocklistIds.push(id); + } + } else if (reason === 'blocklists') { + hasGenericBlocklist = true; + } else if (reason === 'blocklists_subdomains_rule') { + hasSubdomain = true; + } else if (reason.startsWith(SERVICE_PREFIX)) { + const id = reason.slice(SERVICE_PREFIX.length); + if (!serviceIdSet.has(id)) { + serviceIdSet.add(id); + serviceIds.push(id); + } + } else if (reason === 'services') { + hasGenericService = true; + } else if (reason === 'custom_rules') { + hasCustomRule = true; + } else if (reason === 'default_rule') { + hasDefault = true; + } + // Unknown tokens are ignored. + } + + const chips: FormattedReason[] = []; + const subdomainSuffix = hasSubdomain ? ' (subdomain)' : ''; + + // DNSSEC validation failure — shown first; explains an otherwise-opaque SERVFAIL. + if (hasDnssecFailed) { + chips.push({ kind: 'dnssec', label: 'DNSSEC validation failed' }); + } + + // Blocklist tier — specific ids collapse the generic token; the subdomain + // qualifier folds into the chip label rather than becoming its own chip. + if (blocklistIds.length > 0) { + for (const id of blocklistIds) { + const name = blocklistNames?.[id] ?? id; + chips.push({ kind: 'blocklist', label: `Blocklist: ${name}${subdomainSuffix}` }); + } + } else if (hasGenericBlocklist || hasSubdomain) { + // Generic blocklist, or an orphan subdomain rule with nothing to attach to. + chips.push({ kind: 'blocklist', label: `Blocklist${subdomainSuffix}` }); + } + + // Service tier — specific ids collapse the generic token. + if (serviceIds.length > 0) { + for (const id of serviceIds) { + const name = serviceNames?.[id] ?? id; + chips.push({ kind: 'service', label: `Service: ${name}` }); + } + } else if (hasGenericService) { + chips.push({ kind: 'service', label: 'Service' }); + } + + if (hasCustomRule) { + chips.push({ kind: 'custom_rule', label: 'Custom rule' }); + } + + if (hasDefault) { + chips.push({ kind: 'default', label: 'Default rule' }); + } + + return chips; +} + +export default formatReasons; diff --git a/app/src/lib/utils.ts b/app/src/lib/utils.ts index bd0c391d..6227b49a 100644 --- a/app/src/lib/utils.ts +++ b/app/src/lib/utils.ts @@ -4,3 +4,7 @@ import { twMerge } from "tailwind-merge" export function cn(...inputs: ClassValue[]) { return twMerge(clsx(inputs)) } + +// Subtle "raise/grow on hover" affordance shared by interactive cards (setup platform cards, query-log rows). +export const INTERACTIVE_CARD = + "transition-all duration-300 cursor-pointer hover:scale-[1.02] active:scale-100 motion-reduce:transform-none motion-reduce:transition-none"; diff --git a/app/src/pages/logs/Logs.tsx b/app/src/pages/logs/Logs.tsx index 08043a9f..26a564eb 100644 --- a/app/src/pages/logs/Logs.tsx +++ b/app/src/pages/logs/Logs.tsx @@ -14,6 +14,7 @@ import QuickRuleSheet, { type QuickRuleAction } from "./QuickRuleSheet"; import api from "@/api/api"; import { useAppStore } from "@/store/general"; import { Skeleton } from "@/components/ui/skeleton"; +import { Info, X } from "lucide-react"; import { useScreenDetector } from "@/hooks/useScreenDetector"; import { useSubscriptionGuard } from "@/hooks/useSubscriptionGuard"; import LimitedAccessBanner from "@/components/LimitedAccessBanner"; @@ -51,6 +52,20 @@ const QueryLogs = ({ profiles }: QueryLogsProps): JSX.Element => { // Maintain a separate list of all available device IDs (not filtered by current selection) const [allAvailableDeviceIds, setAllAvailableDeviceIds] = useState([]); + // id→name catalogs for enriching query-log reasons (blocklist/service ids). Loaded once on + // mount; failures degrade gracefully to raw ids and must never block logs from rendering. + const [blocklistNames, setBlocklistNames] = useState>({}); + const [serviceNames, setServiceNames] = useState>({}); + + // One-time mobile hint teaching that a row is tappable (there is no visible chevron). + // Dismissed on the ✕ or after the first row expand. Persisted in the shared "moddns-storage" + // zustand store (alongside the other one-time dismissals) so it never reappears. + const expandHintDismissed = useAppStore((state) => state.logsExpandHintDismissed); + const setLogsExpandHintDismissed = useAppStore((state) => state.setLogsExpandHintDismissed); + const dismissExpandHint = useCallback(() => { + setLogsExpandHintDismissed(true); + }, [setLogsExpandHintDismissed]); + // Compose filters object for API const filters = { Limit: QUERY_LIMIT, @@ -97,6 +112,37 @@ const QueryLogs = ({ profiles }: QueryLogsProps): JSX.Element => { // eslint-disable-next-line react-hooks/exhaustive-deps -- activeProfile is intentionally excluded to avoid re-running this effect when the profile object changes (which this effect itself triggers via setActiveProfile) }, [profiles, setActiveProfile]); + // Load blocklist + service catalogs once to resolve reason ids to human names in the + // expandable log card. Best-effort: on failure the maps stay empty and reasons fall back + // to raw ids — never block logs on catalog load. + useEffect(() => { + let cancelled = false; + const loadCatalogs = async () => { + try { + const [blocklistsResp, servicesResp] = await Promise.all([ + api.Client.blocklistsApi.apiV1BlocklistsGet(), + api.Client.servicesApi.apiV1ServicesGet(), + ]); + if (cancelled) return; + const blMap: Record = {}; + (blocklistsResp.data || []).forEach(bl => { + if (bl.blocklist_id) blMap[bl.blocklist_id] = bl.name; + }); + setBlocklistNames(blMap); + + const svcMap: Record = {}; + (servicesResp.data?.services || []).forEach(svc => { + if (svc.id && svc.name) svcMap[svc.id] = svc.name; + }); + setServiceNames(svcMap); + } catch { + // Leave maps empty; reasons degrade to raw ids. + } + }; + loadCatalogs(); + return () => { cancelled = true; }; + }, []); + const handleOpenQuickRule = useCallback((domain?: string, defaultAction: QuickRuleAction = "denylist") => { if (!domain) return; if (isRestricted) return; // POST custom_rules is blocked in Limited Access / Pending Delete @@ -406,7 +452,25 @@ const QueryLogs = ({ profiles }: QueryLogsProps): JSX.Element => { : "Pull to refresh"} )} -
+
+ {!expandHintDismissed && logs.length > 0 && ( +
+ + Tap any entry to see full request details. + +
+ )} {logs.map((log, index) => { const isLast = index === logs.length - 1; return ( @@ -417,6 +481,9 @@ const QueryLogs = ({ profiles }: QueryLogsProps): JSX.Element => { lastLogRef={isLast ? lastLogRef : undefined} onQuickRule={handleOpenQuickRule} quickRuleRestricted={isRestricted} + blocklistNames={blocklistNames} + serviceNames={serviceNames} + onExpand={dismissExpandHint} /> ); })} diff --git a/app/src/pages/logs/QueryLogCard.tsx b/app/src/pages/logs/QueryLogCard.tsx index 59832b95..ea240db2 100644 --- a/app/src/pages/logs/QueryLogCard.tsx +++ b/app/src/pages/logs/QueryLogCard.tsx @@ -1,4 +1,4 @@ -import { useState, type JSX } from "react"; +import { useId, useState, type JSX } from "react"; import { useScreenDetector } from "@/hooks/useScreenDetector"; import { formatDistanceToNow, parseISO, format } from "date-fns"; import { Clock, ShieldPlus } from "lucide-react"; @@ -6,6 +6,8 @@ import { Clock, ShieldPlus } from "lucide-react"; import { Badge } from "@/components/ui/badge"; // still used for Blocked status only import { Button } from "@/components/ui/button"; import { Tooltip } from "@/components/ui/tooltip"; +import { ReasonBadges } from "@/components/ui/ReasonBadges"; +import { cn, INTERACTIVE_CARD } from "@/lib/utils"; import type { ModelQueryLog } from "@/api/client"; interface QueryLogCardProps { @@ -14,9 +16,13 @@ interface QueryLogCardProps { lastLogRef?: (node: HTMLDivElement | null) => void; onQuickRule?: (domain?: string, defaultAction?: "denylist" | "allowlist") => void; quickRuleRestricted?: boolean; + blocklistNames?: Record; + serviceNames?: Record; + /** Called the first time this row is expanded (used to dismiss the one-time mobile hint). */ + onExpand?: () => void; } -const QueryLogCard = ({ log, isLast, lastLogRef, onQuickRule, quickRuleRestricted }: QueryLogCardProps): JSX.Element | null => { +const QueryLogCard = ({ log, isLast, lastLogRef, onQuickRule, quickRuleRestricted, blocklistNames, serviceNames, onExpand }: QueryLogCardProps): JSX.Element | null => { // If domain logging is disabled, dns_request.domain may be absent. Provide a placeholder. const rawDomain = log.dns_request?.domain; const normalizedDomain = rawDomain ? rawDomain.replace(/\.$/, "") : undefined; @@ -38,6 +44,8 @@ const QueryLogCard = ({ log, isLast, lastLogRef, onQuickRule, quickRuleRestricte : isProcessed ? "bg-[var(--tailwind-colors-slate-800)] text-[var(--tailwind-colors-slate-100)] hover:!bg-[var(--tailwind-colors-red-600)] hover:!text-[var(--tailwind-colors-slate-50)]" : "bg-[var(--tailwind-colors-rdns-600)] text-[var(--tailwind-colors-slate-900)] hover:!bg-[var(--tailwind-colors-slate-900)] hover:!text-[var(--tailwind-colors-rdns-600)]"; + // Quick-rule is the ONLY control excluded from the whole-card expand overlay; its wrapper + // sits above the overlay (relative z-20) so it stays clickable. const renderQuickRuleButton = (wrapperClassName: string) => (
@@ -50,21 +58,28 @@ const QueryLogCard = ({ log, isLast, lastLogRef, onQuickRule, quickRuleRestricte aria-label="Quick custom rule" onClick={handleQuickRule} disabled={quickRuleDisabled} - className={`h-9 w-9 lg:min-h-0 p-0 aspect-square rounded-full disabled:opacity-40 ${quickRuleButtonClasses}`} + className={`h-11 w-11 md:h-9 md:w-9 min-h-0 p-0 aspect-square rounded-full disabled:opacity-40 ${quickRuleButtonClasses}`} data-testid="logs-quick-rule-button" > - +
); - // Track timestamp expansion to increase card height smoothly on mobile - const [timestampExpanded, setTimestampExpanded] = useState(false); - - // Expansion state for mobile tap-to-expand of truncated domain (device id no longer truncates) - const [showFullDomainMobile, setShowFullDomainMobile] = useState(false); + // Whole-card expand: every row is expandable (blocked and processed, with or without reasons). + // There is no visible chevron — expandability is signalled by the hover lift (desktop), + // the press/active feedback (both), and a one-time hint (mobile, owned by the Logs page). + const reasons = log.reasons ?? []; + const hasReasons = reasons.length > 0; + const [expanded, setExpanded] = useState(false); + const panelId = useId(); + const toggleExpanded = () => setExpanded(v => { + const next = !v; + if (next) onExpand?.(); + return next; + }); // Device ID: backend allows up to 36 chars; truncate only for mobile (<=768px) const { isMobile } = useScreenDetector(); @@ -75,24 +90,86 @@ const QueryLogCard = ({ log, isLast, lastLogRef, onQuickRule, quickRuleRestricte else deviceIdOrIp = rawDeviceId.slice(0, 36); const DOMAIN_TRUNCATE_THRESHOLD = 65; // existing logic threshold - const MOBILE_EXPANDED_DOMAIN_LIMIT = 50; - const TIMESTAMP_COLLAPSED_MAX_HEIGHT = 24; - const TIMESTAMP_EXPANDED_MAX_HEIGHT = 48; const isDomainTruncatable = displayDomain ? displayDomain.length > DOMAIN_TRUNCATE_THRESHOLD : false; const truncatedDomain = displayDomain && isDomainTruncatable ? displayDomain.slice(0, DOMAIN_TRUNCATE_THRESHOLD) + '…' : displayDomain; - const mobileExpandedDomain = displayDomain - ? displayDomain.length > MOBILE_EXPANDED_DOMAIN_LIMIT - ? displayDomain.slice(0, MOBILE_EXPANDED_DOMAIN_LIMIT) + '…' - : displayDomain - : undefined; const protocolLabel = log?.protocol ? log.protocol.toUpperCase() : '—'; + // DNSSEC status shown inline next to the protocol as a plain text badge (styled like the + // protocol label — no outline/background): + // - validated (AD bit true) -> brand-coloured "DNSSEC" + // - failed (bogus/misconfigured) -> red "DNSSEC" (recursor SERVFAILed on validation) + // Neither shows for domains without a DNSSEC signal. + const dnssecValidated = log.dns_request?.dnssec === true; + const dnssecFailed = reasons.includes('dnssec_failed'); + const dnssecShown = dnssecValidated || dnssecFailed; + // When reserveWhenHidden is set (desktop), the badge is always rendered — invisible + // when there's no DNSSEC — so it reserves a constant slot and the protocol label + // never shifts depending on whether DNSSEC is shown. The testid/color are only + // applied when actually shown. + const renderDnssecBadge = (className?: string, reserveWhenHidden = false) => { + if (!dnssecShown && !reserveWhenHidden) return null; + return ( + + DNSSEC + + ); + }; + + // Detail-grid field: uppercase micro-label + selectable value (optionally coloured). + const renderDetailField = (label: string, value: string, testid: string, valueClassName?: string) => ( +
+
{label}
+
{value}
+
+ ); + + // DNSSEC has three distinct states — keep them clearly worded and colour-coded: + // failed (bogus) -> "Validation failed" (red) — signatures broken + // validated (AD=1) -> "Validated" (brand/green) — authentic + // unsigned -> "No DNSSEC" (muted) — domain isn't signed + const dnssecDetail = dnssecFailed + ? { text: 'Validation failed', className: 'text-[var(--tailwind-colors-red-600)]' } + : dnssecValidated + ? { text: 'Validated', className: 'text-[var(--tailwind-colors-rdns-600)]' } + : { text: 'No DNSSEC', className: 'text-[var(--tailwind-colors-slate-200)]' }; + return (
-
+ {/* Whole-card expand/collapse trigger: a real button (native keyboard/focus/aria). + Spans the ENTIRE card (absolute inset-0 on the card root), so clicking anywhere — + the header row OR the expanded detail panel — toggles it. Collapsed, the panel is + 0-height so the button only covers the header. Quick-rule (z-20) stays above it. */} + - ) : ( - {displayDomain} - ) + + {isDomainTruncatable ? truncatedDomain : displayDomain} + ) : ( '-' )}
-
-
- -
+
+
@@ -185,6 +240,7 @@ const QueryLogCard = ({ log, isLast, lastLogRef, onQuickRule, quickRuleRestricte
{protocolLabel}
+ {renderDnssecBadge("order-2 md:order-2", true)} Blocked @@ -198,46 +254,66 @@ const QueryLogCard = ({ log, isLast, lastLogRef, onQuickRule, quickRuleRestricte {deviceIdOrIp}
- + - {renderQuickRuleButton("flex items-center justify-center")} + {renderQuickRuleButton("flex items-center justify-center relative z-20")} )} +
+
+
+
+ {normalizedDomain !== undefined + ? renderDetailField("Domain", normalizedDomain, "querylog-detail-domain") + : ( +
+
Domain
+
Domain logging disabled
+
+ )} + {log.dns_request?.query_type && renderDetailField("Query type", log.dns_request.query_type, "querylog-detail-query-type")} + {log.dns_request?.response_code && renderDetailField("Response code", log.dns_request.response_code, "querylog-detail-response-code")} + {(log.dns_request?.dnssec !== undefined || dnssecFailed) && renderDetailField("DNSSEC", dnssecDetail.text, "querylog-detail-dnssec", dnssecDetail.className)} + {renderDetailField("Protocol", protocolLabel, "querylog-detail-protocol")} + {log.client_ip && renderDetailField("Client IP", log.client_ip, "querylog-detail-client-ip")} + {log.device_id && renderDetailField("Device ID", log.device_id, "querylog-detail-device-id")} + {renderDetailField("Time", log.timestamp ? format(parseISO(log.timestamp), "MMMM d, yyyy 'at' hh:mm:ss a") : "—", "querylog-detail-timestamp")} +
+ {hasReasons && ( +
+ {(isBlocked || dnssecFailed) ? "Block reason" : "Allow reason"} + +
+ )} +
+
+
); }; -interface TimestampDisplayProps { timestamp?: string; onToggle?: (expanded: boolean) => void } +interface TimestampDisplayProps { timestamp?: string } -const TimestampDisplay = ({ timestamp, onToggle }: TimestampDisplayProps) => { - const [expanded, setExpanded] = useState(false); +// Static relative-time label (Clock icon + "x ago"). The absolute timestamp moves to the panel. +const TimestampDisplay = ({ timestamp }: TimestampDisplayProps) => { if (!timestamp) return null; - const date = parseISO(timestamp); - const relative = formatDistanceToNow(date, { addSuffix: true }); - const absolute = format(date, "MMMM d, yyyy 'at' hh:mm:ss a"); + const relative = formatDistanceToNow(parseISO(timestamp), { addSuffix: true }); return ( - + + {relative} + ); }; diff --git a/app/src/pages/setup/SetupScreen.tsx b/app/src/pages/setup/SetupScreen.tsx index f5508682..afb3edee 100644 --- a/app/src/pages/setup/SetupScreen.tsx +++ b/app/src/pages/setup/SetupScreen.tsx @@ -24,6 +24,7 @@ import VerificationBanner from '@/pages/setup/VerificationBanner'; import modDNSLogoDarkTheme from '@/assets/logos/modDNS-dark-theme.svg'; import modDNSLogoLightTheme from '@/assets/logos/modDNS-light-theme.svg'; import { useTheme } from "@/components/theme-provider"; +import { cn, INTERACTIVE_CARD } from "@/lib/utils"; import SetupGuidePanel from './RightPanelGuide'; @@ -235,13 +236,18 @@ export default function Setup({ profiles }: SetupProps): JSX.Element { handlePlatformClick(platform.name)} > @@ -259,10 +265,13 @@ export default function Setup({ profiles }: SetupProps): JSX.Element { {/* Device Identification Card - full width */} handlePlatformClick('Device Identification')} > @@ -300,10 +309,13 @@ export default function Setup({ profiles }: SetupProps): JSX.Element { handlePlatformClick(platform.name)} > @@ -318,10 +330,13 @@ export default function Setup({ profiles }: SetupProps): JSX.Element { ))} handlePlatformClick('Device Identification')} > diff --git a/app/src/store/general.ts b/app/src/store/general.ts index e4d01fd0..98e25502 100644 --- a/app/src/store/general.ts +++ b/app/src/store/general.ts @@ -21,6 +21,8 @@ interface AppState { setBlocklistsAlertDismissed: (dismissed: boolean) => void; customRulesAlertDismissed: boolean; // session-only dismissal (not persisted) setCustomRulesAlertDismissed: (dismissed: boolean) => void; + logsExpandHintDismissed: boolean; // persisted dismissal of the one-time "tap a row" logs hint + setLogsExpandHintDismissed: (dismissed: boolean) => void; passkeys: ModelCredential[]; setPasskeys: (passkeys: ModelCredential[]) => void; subscriptionStatus: string | null; @@ -79,6 +81,8 @@ export const useAppStore = create()( setBlocklistsAlertDismissed: (dismissed) => set({ blocklistsAlertDismissed: dismissed }), customRulesAlertDismissed: false, setCustomRulesAlertDismissed: (dismissed) => set({ customRulesAlertDismissed: dismissed }), + logsExpandHintDismissed: false, + setLogsExpandHintDismissed: (dismissed) => set({ logsExpandHintDismissed: dismissed }), passkeys: [], setPasskeys: (passkeys) => set({ passkeys }), subscriptionStatus: null, @@ -104,6 +108,7 @@ export const useAppStore = create()( connectionStatusVisible: state.connectionStatusVisible, announcementsLastSeenAt: state.announcementsLastSeenAt, customRulesCollapsed: state.customRulesCollapsed, + logsExpandHintDismissed: state.logsExpandHintDismissed, }), } ) diff --git a/proxy/cache/memory/serialization_test.go b/proxy/cache/memory/serialization_test.go index b77a4053..48d8f147 100644 --- a/proxy/cache/memory/serialization_test.go +++ b/proxy/cache/memory/serialization_test.go @@ -40,6 +40,10 @@ func TestRequestContextSerialization(t *testing.T) { assert.Equal(t, "test-profile", reqCtx.LoggerConfig.ProfileID, "Logger config should have correct profile ID") assert.False(t, reqCtx.LoggerConfig.Enabled, "Logger config should show enabled=false") + // UpstreamName must survive the cache round-trip — EmitQueryLog needs it to pick + // the recursor for the DNSSEC-failure CD probe. Regression guard: it was json:"-". + reqCtx.UpstreamName = "knot" + // Test serialization by setting in cache requestID := "test-request-123" err = profileIDCache.SetRequestCtx(requestID, reqCtx) @@ -55,6 +59,7 @@ func TestRequestContextSerialization(t *testing.T) { assert.Equal(t, map[string]string{"privacy": "setting"}, retrievedCtx.PrivacySettings, "Privacy settings should be preserved") assert.Equal(t, map[string]string{"dnssec": "enabled"}, retrievedCtx.DNSSECSettings, "DNSSEC settings should be preserved") assert.Equal(t, map[string]string{"advanced": "setting"}, retrievedCtx.AdvancedSettings, "Advanced settings should be preserved") + assert.Equal(t, "knot", retrievedCtx.UpstreamName, "UpstreamName must survive the cache round-trip (needed by the DNSSEC-failure probe)") // Verify the logger is recreated correctly require.NotNil(t, retrievedCtx.Logger, "Logger should be recreated") diff --git a/proxy/internal/dnssec/dnssec.go b/proxy/internal/dnssec/dnssec.go new file mode 100644 index 00000000..57178a8a --- /dev/null +++ b/proxy/internal/dnssec/dnssec.go @@ -0,0 +1,132 @@ +// Package dnssec holds the proxy's DNSSEC request/response helpers: setting the +// request flags that make recursors return the Authenticated Data flag and +// Extended DNS Errors, and capturing/classifying those EDE codes so a DNSSEC +// validation failure can be surfaced on the query log. +package dnssec + +import ( + "sync" + + "github.com/AdguardTeam/dnsproxy/upstream" + "github.com/miekg/dns" +) + +// ReasonFailed is appended to a query log's reasons when the recursor reports a +// DNSSEC validation failure via an Extended DNS Error (RFC 8914). The frontend +// renders it as a "DNSSEC validation failed" chip. +const ReasonFailed = "dnssec_failed" + +// ApplyRequestFlags configures the upstream request's DNSSEC-related bits. +// +// The logged DNSSEC-validation status (QueryLog.DNSRequest.DNSSEC, sourced from the +// response AD bit) is deliberately decoupled from the client-facing send_do_bit +// setting: validation happens at the recursor regardless of whether DNSSEC RRs are +// returned to the end device. +// - validation enabled -> set the request AD bit so the recursor returns, and the +// dnsproxy library preserves (filterMsg keeps AD when the request's AD or DO bit +// is set), the Authenticated Data flag — even when the DO bit is not sent. +// - validation disabled -> set CD (CheckingDisabled) so the recursor skips validation. +// +// EDNS(0) is attached whenever validation is enabled — so the recursor can return +// Extended DNS Errors (carried in the OPT record) on validation failure, which +// happens whenever the query carries EDNS, independent of the DO bit — or when the +// client asked for DNSSEC RRs (sendDoBit). The DO bit, set to sendDoBit, governs +// returning RRSIG/DNSKEY records to the client. +func ApplyRequestFlags(req *dns.Msg, dnssecEnabled, sendDoBit bool) { + req.Extra = make([]dns.RR, 0) + if dnssecEnabled { + req.AuthenticatedData = true + } else { + req.CheckingDisabled = true + } + + if dnssecEnabled || sendDoBit { + req.SetEdns0(2048, sendDoBit) + } +} + +// IsFailureEDE reports whether an EDE InfoCode denotes a DNSSEC *validation +// failure* (bogus zone), as opposed to merely insecure/indeterminate. RFC 8914: +// +// 6 DNSSEC Bogus, 7 Signature Expired, 8 Signature Not Yet Valid, +// 9 DNSKEY Missing, 10 RRSIGs Missing, 11 No Zone Key Bit Set, 12 NSEC Missing. +// +// Codes 1/2/5 (unsupported algorithm/digest, indeterminate) mean the zone is +// treated as insecure, not failed, so they are deliberately excluded — an +// unsigned/insecure domain must never be flagged. Verified against sdns and +// knot-resolver v6.4.0, which both emit codes in this range on SERVFAIL. +func IsFailureEDE(code uint16) bool { + return code >= 6 && code <= 12 +} + +// FailureEDE returns the first DNSSEC-failure EDE InfoCode found in msg's OPT +// record, if any. +func FailureEDE(msg *dns.Msg) (uint16, bool) { + if msg == nil { + return 0, false + } + opt := msg.IsEdns0() + if opt == nil { + return 0, false + } + for _, o := range opt.Option { + if ede, ok := o.(*dns.EDNS0_EDE); ok && IsFailureEDE(ede.InfoCode) { + return ede.InfoCode, true + } + } + return 0, false +} + +// EDEStore correlates a captured DNSSEC-failure EDE code with the request that +// produced it, keyed by the request *dns.Msg pointer. dnsproxy passes the same +// dctx.Req pointer to the upstream Exchange and later exposes it to EmitQueryLog, +// so the pointer is a stable per-request key. Entries are set by CapturingUpstream +// at exchange time and drained by EmitQueryLog. Only DNSSEC-failure responses store +// an entry, so the map stays tiny and short-lived. +type EDEStore struct{ m sync.Map } + +// Set records the EDE code for req. +func (s *EDEStore) Set(req *dns.Msg, code uint16) { + if s == nil { + return + } + s.m.Store(req, code) +} + +// Take returns and removes the stored EDE code for req. Nil-safe so a caller +// constructed without an EDEStore (e.g. in unit tests) is a harmless no-op. +func (s *EDEStore) Take(req *dns.Msg) (uint16, bool) { + if s == nil { + return 0, false + } + v, ok := s.m.LoadAndDelete(req) + if !ok { + return 0, false + } + return v.(uint16), true +} + +// CapturingUpstream wraps an upstream to capture DNSSEC-failure EDE codes from +// responses BEFORE dnsproxy's filterMsg strips the OPT record (which happens +// before the query log is emitted, so the EDE is otherwise unavailable at log +// time). Address()/Close() come from the embedded upstream; only Exchange is +// intercepted. +type CapturingUpstream struct { + upstream.Upstream + store *EDEStore +} + +// NewCapturingUpstream wraps u so DNSSEC-failure EDE codes are captured into store. +func NewCapturingUpstream(u upstream.Upstream, store *EDEStore) *CapturingUpstream { + return &CapturingUpstream{Upstream: u, store: store} +} + +func (u *CapturingUpstream) Exchange(req *dns.Msg) (*dns.Msg, error) { + resp, err := u.Upstream.Exchange(req) + if err == nil { + if code, ok := FailureEDE(resp); ok { + u.store.Set(req, code) + } + } + return resp, err +} diff --git a/proxy/internal/dnssec/dnssec_test.go b/proxy/internal/dnssec/dnssec_test.go new file mode 100644 index 00000000..5b93154b --- /dev/null +++ b/proxy/internal/dnssec/dnssec_test.go @@ -0,0 +1,185 @@ +package dnssec + +import ( + "errors" + "testing" + + "github.com/miekg/dns" + "github.com/stretchr/testify/assert" +) + +// mockUpstream implements upstream.Upstream for wrapper tests. +type mockUpstream struct { + resp *dns.Msg + err error + gotReq *dns.Msg +} + +func (m *mockUpstream) Exchange(req *dns.Msg) (*dns.Msg, error) { + m.gotReq = req + return m.resp, m.err +} +func (m *mockUpstream) Address() string { return "mock" } +func (m *mockUpstream) Close() error { return nil } + +// msgWithEDE builds a response carrying an OPT record with the given EDE InfoCode. +func msgWithEDE(rcode int, code uint16) *dns.Msg { + m := new(dns.Msg) + m.SetQuestion(dns.Fqdn("dnssec-failed.org"), dns.TypeA) + m.Rcode = rcode + opt := &dns.OPT{Hdr: dns.RR_Header{Name: ".", Rrtype: dns.TypeOPT}} + opt.Option = append(opt.Option, &dns.EDNS0_EDE{InfoCode: code}) + m.Extra = append(m.Extra, opt) + return m +} + +func newReq() *dns.Msg { + req := new(dns.Msg) + req.SetQuestion(dns.Fqdn("example.com"), dns.TypeA) + // seed Extra to confirm it is reset + req.Extra = []dns.RR{&dns.TXT{Hdr: dns.RR_Header{Name: "x.", Rrtype: dns.TypeTXT}, Txt: []string{"seed"}}} + return req +} + +// ApplyRequestFlags decouples logged validation status from the client-facing +// send_do_bit and always attaches EDNS when validation is enabled so the recursor +// can return EDE. +func TestApplyRequestFlags(t *testing.T) { + t.Run("enabled, send_do_bit off: AD set, no CD, EDNS present but DO=0", func(t *testing.T) { + req := newReq() + ApplyRequestFlags(req, true, false) + assert.True(t, req.AuthenticatedData, "AD bit must be set so validation is logged") + assert.False(t, req.CheckingDisabled) + if o := req.IsEdns0(); assert.NotNil(t, o, "EDNS(0) must be present so EDE can be returned") { + assert.False(t, o.Do(), "DO must be off when send_do_bit is off") + } + }) + + t.Run("enabled, send_do_bit on: AD set and DO set", func(t *testing.T) { + req := newReq() + ApplyRequestFlags(req, true, true) + assert.True(t, req.AuthenticatedData) + assert.False(t, req.CheckingDisabled) + if o := req.IsEdns0(); assert.NotNil(t, o) { + assert.True(t, o.Do()) + } + }) + + t.Run("disabled: CD set, AD not set, no EDNS", func(t *testing.T) { + req := newReq() + ApplyRequestFlags(req, false, false) + assert.True(t, req.CheckingDisabled, "CD must be set so the recursor skips validation") + assert.False(t, req.AuthenticatedData) + assert.Nil(t, req.IsEdns0(), "no EDNS when validation is disabled") + }) + + t.Run("disabled, send_do_bit on: CD set, DO set, AD not set", func(t *testing.T) { + req := newReq() + ApplyRequestFlags(req, false, true) + assert.True(t, req.CheckingDisabled) + assert.False(t, req.AuthenticatedData) + if o := req.IsEdns0(); assert.NotNil(t, o) { + assert.True(t, o.Do()) + } + }) + + t.Run("Extra is reset (seed cleared)", func(t *testing.T) { + req := newReq() + ApplyRequestFlags(req, true, false) + for _, rr := range req.Extra { + _, isTXT := rr.(*dns.TXT) + assert.False(t, isTXT, "seeded/stale RRs must be cleared") + } + }) +} + +func TestIsFailureEDE(t *testing.T) { + // DNSSEC validation-failure codes 6..12 are failures. + for _, c := range []uint16{6, 7, 8, 9, 10, 11, 12} { + assert.True(t, IsFailureEDE(c), "code %d should be a DNSSEC failure", c) + } + // Insecure/indeterminate/other codes must NOT be treated as failures + // (so unsigned domains are never flagged). + for _, c := range []uint16{0, 1, 2, 3, 4, 5, 13, 29} { + assert.False(t, IsFailureEDE(c), "code %d should NOT be a DNSSEC failure", c) + } +} + +// tableRef: logs-reason-display-behaviour #13 +func TestFailureEDE(t *testing.T) { + t.Run("SERVFAIL with EDE 9 -> detected", func(t *testing.T) { + code, ok := FailureEDE(msgWithEDE(dns.RcodeServerFailure, 9)) + assert.True(t, ok) + assert.Equal(t, uint16(9), code) + }) + t.Run("EDE 5 (indeterminate) -> not a failure", func(t *testing.T) { + _, ok := FailureEDE(msgWithEDE(dns.RcodeServerFailure, 5)) + assert.False(t, ok) + }) + t.Run("no OPT/EDE -> not a failure", func(t *testing.T) { + m := new(dns.Msg) + m.SetQuestion(dns.Fqdn("example.com"), dns.TypeA) + _, ok := FailureEDE(m) + assert.False(t, ok) + _, ok = FailureEDE(nil) + assert.False(t, ok) + }) +} + +func TestEDEStore(t *testing.T) { + s := &EDEStore{} + req := new(dns.Msg) + req.SetQuestion(dns.Fqdn("x.org"), dns.TypeA) + + _, ok := s.Take(req) + assert.False(t, ok, "empty store returns nothing") + + s.Set(req, 9) + code, ok := s.Take(req) + assert.True(t, ok) + assert.Equal(t, uint16(9), code) + + _, ok = s.Take(req) + assert.False(t, ok, "Take must remove the entry") + + // nil-safe + var ns *EDEStore + ns.Set(req, 9) + _, ok = ns.Take(req) + assert.False(t, ok) +} + +func TestCapturingUpstream(t *testing.T) { + req := new(dns.Msg) + req.SetQuestion(dns.Fqdn("dnssec-failed.org"), dns.TypeA) + + t.Run("captures DNSSEC-failure EDE keyed by request pointer", func(t *testing.T) { + store := &EDEStore{} + u := NewCapturingUpstream(&mockUpstream{resp: msgWithEDE(dns.RcodeServerFailure, 9)}, store) + _, err := u.Exchange(req) + assert.NoError(t, err) + code, ok := store.Take(req) + assert.True(t, ok, "EDE must be captured for the exact request") + assert.Equal(t, uint16(9), code) + }) + + t.Run("no capture for a clean response", func(t *testing.T) { + store := &EDEStore{} + clean := new(dns.Msg) + clean.SetQuestion(dns.Fqdn("cloudflare.com"), dns.TypeA) + clean.Rcode = dns.RcodeSuccess + u := NewCapturingUpstream(&mockUpstream{resp: clean}, store) + _, _ = u.Exchange(req) + _, ok := store.Take(req) + assert.False(t, ok) + }) + + t.Run("no capture on exchange error", func(t *testing.T) { + store := &EDEStore{} + u := NewCapturingUpstream(&mockUpstream{err: errors.New("timeout")}, store) + _, err := u.Exchange(req) + assert.Error(t, err) + _, ok := store.Take(req) + assert.False(t, ok) + }) +} diff --git a/proxy/requestcontext/request_context.go b/proxy/requestcontext/request_context.go index 6ca34ca9..f9bf2272 100644 --- a/proxy/requestcontext/request_context.go +++ b/proxy/requestcontext/request_context.go @@ -23,7 +23,7 @@ type RequestContext struct { Logger logging.LoggerInterface `json:"-"` LoggerConfig logging.LoggingConfig `json:"logger_config"` StartTime time.Time `json:"-"` - UpstreamName string `json:"-"` + UpstreamName string `json:"upstream_name"` } func NewRequestContext(ctx context.Context, p *proxy.Proxy, profileId string, deviceId string, privacySettings, logsSettings, dnssecSettings, advancedSettings map[string]string, logger logging.LoggerInterface) *RequestContext { diff --git a/proxy/server/proxy.go b/proxy/server/proxy.go index e7717ef1..bec3d9f8 100644 --- a/proxy/server/proxy.go +++ b/proxy/server/proxy.go @@ -11,6 +11,7 @@ import ( "github.com/AdguardTeam/golibs/netutil" "github.com/AdguardTeam/golibs/service" "github.com/ivpn/dns/proxy/config" + "github.com/ivpn/dns/proxy/internal/dnssec" "github.com/rs/zerolog/log" ) @@ -67,9 +68,12 @@ func (s *Server) newProxyConfig(serverConfig *config.Config) (*proxy.Config, err if err != nil { return nil, fmt.Errorf("failed to create upstream: %w", err) } + // Wrap the upstream so we can read the DNSSEC-failure EDE code from the + // response before dnsproxy's filterMsg strips the OPT (see EmitQueryLog). + wrappedUps := dnssec.NewCapturingUpstream(ups, s.edeStore) upCfg := &proxy.UpstreamConfig{ Upstreams: []upstream.Upstream{ - ups, + wrappedUps, }, } customUpstreamConfig := proxy.NewCustomUpstreamConfig( diff --git a/proxy/server/query_logs.go b/proxy/server/query_logs.go index a52b55d2..de16f2c8 100644 --- a/proxy/server/query_logs.go +++ b/proxy/server/query_logs.go @@ -6,14 +6,27 @@ import ( "github.com/AdguardTeam/dnsproxy/proxy" "github.com/getsentry/sentry-go" + "github.com/ivpn/dns/proxy/internal/dnssec" "github.com/ivpn/dns/proxy/model" "github.com/ivpn/dns/proxy/requestcontext" "github.com/miekg/dns" ) +// appendReason returns a new slice with r appended, without mutating existing +// (which is shared with the request context's FilterResult). +func appendReason(existing []string, r string) []string { + out := make([]string, len(existing), len(existing)+1) + copy(out, existing) + return append(out, r) +} + func (s *Server) EmitQueryLog(reqCtx *requestcontext.RequestContext, dctx *proxy.DNSContext) { defer sentry.Recover() + // Drain any captured DNSSEC-failure EDE for this request unconditionally (even + // if logging is disabled) so the edeStore never leaks entries. + _, dnssecFailed := s.edeStore.Take(dctx.Req) + // Use the contextual logger from the request context logger := reqCtx.Logger @@ -59,6 +72,10 @@ func (s *Server) EmitQueryLog(reqCtx *requestcontext.RequestContext, dctx *proxy queryLog.DNSRequest.ResponseCode = dns.RcodeToString[dctx.Res.Rcode] queryLog.DNSRequest.DNSSEC = dctx.Res.AuthenticatedData } + + if dnssecFailed { + queryLog.Reasons = appendReason(queryLog.Reasons, dnssec.ReasonFailed) + } retention := model.Retention(logsSettings["retention"]) // send event to channel if sendErr := s.CollectorChannels[model.TYPE_QUERY_LOGS].Send( diff --git a/proxy/server/server.go b/proxy/server/server.go index f40656bb..3b2e4d5e 100644 --- a/proxy/server/server.go +++ b/proxy/server/server.go @@ -19,6 +19,7 @@ import ( "github.com/ivpn/dns/proxy/config" "github.com/ivpn/dns/proxy/filter" "github.com/ivpn/dns/proxy/internal/asnlookup" + "github.com/ivpn/dns/proxy/internal/dnssec" "github.com/ivpn/dns/proxy/internal/metrics" "github.com/ivpn/dns/proxy/internal/ratelimit" "github.com/ivpn/dns/proxy/model" @@ -40,9 +41,12 @@ type RequestManager interface { } type Server struct { - Config *config.Config - Proxy *proxy.Proxy // service.Interface - Upstreams map[string]*proxy.CustomUpstreamConfig + Config *config.Config + Proxy *proxy.Proxy // service.Interface + Upstreams map[string]*proxy.CustomUpstreamConfig + // edeStore holds DNSSEC-failure Extended DNS Error codes captured from upstream + // responses (by dnssec.CapturingUpstream), drained per-request by EmitQueryLog. + edeStore *dnssec.EDEStore DomainFilter filter.Filter IPFilter filter.Filter Cache cache.Cache @@ -96,6 +100,7 @@ func NewServer(serverConfig *config.Config, collectorChannels map[string]channel ProfileSettingsCache: profileSettingsCache, CollectorChannels: collectorChannels, Upstreams: make(map[string]*proxy.CustomUpstreamConfig, 0), + edeStore: &dnssec.EDEStore{}, LoggerFactory: loggerFactory, RateLimiter: rl, Metrics: metrics.NewServerMetrics(prometheus.DefaultRegisterer), @@ -303,16 +308,7 @@ func (s *Server) HandleBefore(p *proxy.Proxy, dctx *proxy.DNSContext) (err error return err } - dctx.Req.Extra = make([]dns.RR, 0) - if !dnssecEnabled { - dctx.Req.CheckingDisabled = true - } - - if sendDoBit { - // Enable EDNS0 with a reasonable UDP buffer size and DO=1 - // This sets a proper OPT RR instead of constructing one manually. - dctx.Req.SetEdns0(2048, true) - } + dnssec.ApplyRequestFlags(dctx.Req, dnssecEnabled, sendDoBit) } return nil