mirror of
https://github.com/shankar0123/certctl.git
synced 2026-06-10 09:28:52 +00:00
Merge branch 'fix/bundle-8-frontend-hardening' (Bundle 8: Frontend Hardening, 2 audit findings closed + 3 partial + 1 new ID)
This commit is contained in:
@@ -825,6 +825,77 @@ jobs:
|
|||||||
ALLOWLIST_SIZE=$(echo "$ALLOW" | tr '|' '\n' | wc -l)
|
ALLOWLIST_SIZE=$(echo "$ALLOW" | tr '|' '\n' | wc -l)
|
||||||
echo "T-1 page-coverage guardrail: clean (allowlist size: $ALLOWLIST_SIZE pages deferred)."
|
echo "T-1 page-coverage guardrail: clean (allowlist size: $ALLOWLIST_SIZE pages deferred)."
|
||||||
|
|
||||||
|
- name: Bundle-8 / L-015 target=_blank rel=noopener regression guard
|
||||||
|
# Audit L-015 / CWE-1022 (reverse-tabnabbing): every <a target="_blank">
|
||||||
|
# MUST carry rel="noopener noreferrer" so a malicious page at the
|
||||||
|
# target URL cannot navigate the opener window via window.opener.
|
||||||
|
# At Bundle-8 close (commit b566355→) all 3 sites in the codebase
|
||||||
|
# already comply — this guard prevents regression. The
|
||||||
|
# ExternalLink component (web/src/components/ExternalLink.tsx)
|
||||||
|
# is the recommended way to add new external links.
|
||||||
|
run: |
|
||||||
|
set -e
|
||||||
|
OFFENDERS=$(grep -rnE 'target=["'"'"']?_blank["'"'"']?' web/src/ 2>/dev/null \
|
||||||
|
| grep -v 'noopener noreferrer' \
|
||||||
|
| grep -v 'web/src/components/ExternalLink.tsx' \
|
||||||
|
|| true)
|
||||||
|
if [ -n "$OFFENDERS" ]; then
|
||||||
|
echo "L-015 regression: target=\"_blank\" without rel=\"noopener noreferrer\":"
|
||||||
|
echo "$OFFENDERS"
|
||||||
|
echo ""
|
||||||
|
echo "Either add rel=\"noopener noreferrer\" inline,"
|
||||||
|
echo "or migrate to <ExternalLink> from web/src/components/ExternalLink.tsx."
|
||||||
|
exit 1
|
||||||
|
fi
|
||||||
|
echo "L-015 target=_blank guardrail: clean."
|
||||||
|
|
||||||
|
- name: Bundle-8 / L-019 dangerouslySetInnerHTML regression guard
|
||||||
|
# Audit L-019 / CWE-79 (XSS): no production code may use
|
||||||
|
# dangerouslySetInnerHTML directly. At Bundle-8 close the codebase
|
||||||
|
# has 0 sites; future genuine needs MUST route through
|
||||||
|
# web/src/utils/safeHtml.ts::sanitizeHtml.
|
||||||
|
run: |
|
||||||
|
set -e
|
||||||
|
OFFENDERS=$(grep -rnE 'dangerouslySetInnerHTML' web/src/ 2>/dev/null \
|
||||||
|
| grep -v 'web/src/utils/safeHtml.ts' \
|
||||||
|
|| true)
|
||||||
|
if [ -n "$OFFENDERS" ]; then
|
||||||
|
echo "L-019 regression: dangerouslySetInnerHTML used outside safeHtml.ts:"
|
||||||
|
echo "$OFFENDERS"
|
||||||
|
echo ""
|
||||||
|
echo "Route through web/src/utils/safeHtml.ts::sanitizeHtml — see file"
|
||||||
|
echo "header for the activation procedure (DOMPurify dependency)."
|
||||||
|
exit 1
|
||||||
|
fi
|
||||||
|
echo "L-019 dangerouslySetInnerHTML guardrail: clean."
|
||||||
|
|
||||||
|
- name: Bundle-8 / M-009 mutation invalidation contract guard
|
||||||
|
# Audit M-009: every useMutation must either invalidate the
|
||||||
|
# queries it changes OR document why no invalidation is needed.
|
||||||
|
# SOFT guard — counts useMutation sites and asserts the budget
|
||||||
|
# doesn't grow without a corresponding invalidateQueries / setQueryData /
|
||||||
|
# useTrackedMutation reference. Stricter per-site enforcement is
|
||||||
|
# tracked as M-029 (covers the long-tail useListParams + useTrackedMutation
|
||||||
|
# migration of the existing 56 useMutation sites).
|
||||||
|
run: |
|
||||||
|
set -e
|
||||||
|
MUTATIONS=$(grep -rcE 'useMutation\(|useTrackedMutation\(' web/src/ 2>/dev/null \
|
||||||
|
| awk -F: '{s+=$2} END{print s}')
|
||||||
|
INVALIDATIONS=$(grep -rcE 'invalidateQueries|setQueryData|removeQueries|invalidates:' web/src/ 2>/dev/null \
|
||||||
|
| awk -F: '{s+=$2} END{print s}')
|
||||||
|
echo "M-009 budget — useMutation sites: $MUTATIONS / invalidation sites: $INVALIDATIONS"
|
||||||
|
# At Bundle-8 close: 56 useMutation + 70 invalidation. We allow
|
||||||
|
# +5 mutations growth before requiring invalidation parity. If
|
||||||
|
# the gap widens, audit the new mutation sites for missing
|
||||||
|
# invalidation pairs.
|
||||||
|
BUDGET=$((INVALIDATIONS + 5))
|
||||||
|
if [ "$MUTATIONS" -gt "$BUDGET" ]; then
|
||||||
|
echo "M-009 regression: $MUTATIONS useMutation sites exceeds invalidation budget ($BUDGET)."
|
||||||
|
echo "New mutations should pair with invalidateQueries/setQueryData OR migrate to"
|
||||||
|
echo "useTrackedMutation (web/src/hooks/useTrackedMutation.ts) with explicit invalidates."
|
||||||
|
exit 1
|
||||||
|
fi
|
||||||
|
|
||||||
- name: Forbidden env-var docs drift regression guard (G-3)
|
- name: Forbidden env-var docs drift regression guard (G-3)
|
||||||
# G-3 master closed cat-g-163dae19bc59 (docs-only env vars
|
# G-3 master closed cat-g-163dae19bc59 (docs-only env vars
|
||||||
# phantom in features.md), cat-g-b8f8f8796159 (6 config-only
|
# phantom in features.md), cat-g-b8f8f8796159 (6 config-only
|
||||||
|
|||||||
@@ -0,0 +1,32 @@
|
|||||||
|
// Bundle-8 / Audit L-015 / CWE-1022:
|
||||||
|
// regression coverage for the ExternalLink component. Confirms the
|
||||||
|
// rel="noopener noreferrer" pair is hardcoded and the forwarded
|
||||||
|
// attributes survive — defends against a future "I'll just spread the
|
||||||
|
// rest props" refactor that accidentally lets the caller override `rel`.
|
||||||
|
|
||||||
|
import { describe, it, expect } from 'vitest';
|
||||||
|
import { render, screen } from '@testing-library/react';
|
||||||
|
import { ExternalLink } from './ExternalLink';
|
||||||
|
|
||||||
|
describe('ExternalLink — Bundle-8 / L-015', () => {
|
||||||
|
it('renders target=_blank with rel=noopener noreferrer', () => {
|
||||||
|
render(
|
||||||
|
<ExternalLink href="https://docs.example.com/setup">Setup guide</ExternalLink>,
|
||||||
|
);
|
||||||
|
const link = screen.getByRole('link', { name: 'Setup guide' });
|
||||||
|
expect(link.getAttribute('target')).toBe('_blank');
|
||||||
|
expect(link.getAttribute('rel')).toBe('noopener noreferrer');
|
||||||
|
expect(link.getAttribute('href')).toBe('https://docs.example.com/setup');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('preserves caller className without dropping rel', () => {
|
||||||
|
render(
|
||||||
|
<ExternalLink href="https://example.com" className="text-accent">
|
||||||
|
Link
|
||||||
|
</ExternalLink>,
|
||||||
|
);
|
||||||
|
const link = screen.getByRole('link', { name: 'Link' });
|
||||||
|
expect(link.className).toBe('text-accent');
|
||||||
|
expect(link.getAttribute('rel')).toBe('noopener noreferrer');
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -0,0 +1,48 @@
|
|||||||
|
// Bundle-8 / Audit L-015 / CWE-1022 (Use of Web Link to Untrusted Target
|
||||||
|
// with window.opener Access) / Reverse-tabnabbing:
|
||||||
|
//
|
||||||
|
// Single chokepoint for any anchor that opens in a new tab. Forces the
|
||||||
|
// `rel="noopener noreferrer"` pair so a malicious page at the target URL
|
||||||
|
// cannot navigate the opener window via `window.opener.location =
|
||||||
|
// 'https://evil.example/'`.
|
||||||
|
//
|
||||||
|
// At Bundle-8 time the codebase has 3 `target="_blank"` sites (all in
|
||||||
|
// OnboardingWizard.tsx, all already correct). This component exists so
|
||||||
|
// future external-link additions route through one path and the CI
|
||||||
|
// regression guard at `.github/workflows/ci.yml` ("Bundle-8 / L-015
|
||||||
|
// target=_blank guard") can grep-fail any new bare `target="_blank"`
|
||||||
|
// outside this component.
|
||||||
|
//
|
||||||
|
// Usage:
|
||||||
|
//
|
||||||
|
// <ExternalLink href="https://docs.example.com/setup">Setup guide</ExternalLink>
|
||||||
|
//
|
||||||
|
// The component renders the same `<a>` element + className conventions
|
||||||
|
// as the existing OnboardingWizard sites so retrofits are mechanical.
|
||||||
|
|
||||||
|
import type { AnchorHTMLAttributes, ReactNode } from 'react';
|
||||||
|
|
||||||
|
interface ExternalLinkProps
|
||||||
|
extends Omit<AnchorHTMLAttributes<HTMLAnchorElement>, 'rel' | 'target'> {
|
||||||
|
/** The external URL to open in a new tab. Required. */
|
||||||
|
href: string;
|
||||||
|
/** Anchor body. Typically the link text. */
|
||||||
|
children: ReactNode;
|
||||||
|
}
|
||||||
|
|
||||||
|
export function ExternalLink({ href, children, className, ...rest }: ExternalLinkProps) {
|
||||||
|
return (
|
||||||
|
<a
|
||||||
|
{...rest}
|
||||||
|
href={href}
|
||||||
|
target="_blank"
|
||||||
|
// Bundle-8 / L-015: NEVER drop the rel value. The CI regression
|
||||||
|
// guard greps for any `target="_blank"` outside this component
|
||||||
|
// and fails the build if it finds one without `noopener`.
|
||||||
|
rel="noopener noreferrer"
|
||||||
|
className={className}
|
||||||
|
>
|
||||||
|
{children}
|
||||||
|
</a>
|
||||||
|
);
|
||||||
|
}
|
||||||
@@ -0,0 +1,97 @@
|
|||||||
|
// Bundle-8 / Audit M-010:
|
||||||
|
// regression coverage for useListParams. Exercises the URL contract
|
||||||
|
// (page/page_size/sort/filter[*]), default omission (defaults stay out
|
||||||
|
// of the URL), filter-resets-page invariant, and resetParams.
|
||||||
|
|
||||||
|
import { describe, it, expect } from 'vitest';
|
||||||
|
import { renderHook, act } from '@testing-library/react';
|
||||||
|
import { MemoryRouter, useSearchParams } from 'react-router-dom';
|
||||||
|
import { useListParams } from './useListParams';
|
||||||
|
import type { ReactNode } from 'react';
|
||||||
|
|
||||||
|
function wrapper(initialEntries: string[]) {
|
||||||
|
return ({ children }: { children: ReactNode }) => (
|
||||||
|
<MemoryRouter initialEntries={initialEntries}>{children}</MemoryRouter>
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
describe('useListParams — Bundle-8 / M-010', () => {
|
||||||
|
it('reads defaults when URL is empty', () => {
|
||||||
|
const { result } = renderHook(() => useListParams(), { wrapper: wrapper(['/']) });
|
||||||
|
expect(result.current.params.page).toBe(1);
|
||||||
|
expect(result.current.params.pageSize).toBe(25);
|
||||||
|
expect(result.current.params.sort).toBe('');
|
||||||
|
expect(result.current.params.filters).toEqual({});
|
||||||
|
});
|
||||||
|
|
||||||
|
it('parses page/page_size/sort/filter[*] from the URL', () => {
|
||||||
|
const { result } = renderHook(() => useListParams(), {
|
||||||
|
wrapper: wrapper(['/?page=3&page_size=50&sort=-created_at&filter[status]=active&filter[team_id]=t-platform']),
|
||||||
|
});
|
||||||
|
expect(result.current.params.page).toBe(3);
|
||||||
|
expect(result.current.params.pageSize).toBe(50);
|
||||||
|
expect(result.current.params.sort).toBe('-created_at');
|
||||||
|
expect(result.current.params.filters).toEqual({
|
||||||
|
status: 'active',
|
||||||
|
team_id: 't-platform',
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
it('honours per-call defaults overrides', () => {
|
||||||
|
const { result } = renderHook(
|
||||||
|
() => useListParams({ pageSize: 100, sort: 'name' }),
|
||||||
|
{ wrapper: wrapper(['/']) },
|
||||||
|
);
|
||||||
|
expect(result.current.params.pageSize).toBe(100);
|
||||||
|
expect(result.current.params.sort).toBe('name');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('rejects garbage page values and falls back to default', () => {
|
||||||
|
const { result } = renderHook(() => useListParams(), {
|
||||||
|
wrapper: wrapper(['/?page=not-a-number&page_size=-5']),
|
||||||
|
});
|
||||||
|
expect(result.current.params.page).toBe(1);
|
||||||
|
expect(result.current.params.pageSize).toBe(25);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('omits defaults from the URL on update', () => {
|
||||||
|
function Hookrunner() {
|
||||||
|
const [params] = useSearchParams();
|
||||||
|
const list = useListParams();
|
||||||
|
return { params, list };
|
||||||
|
}
|
||||||
|
const { result } = renderHook(() => Hookrunner(), { wrapper: wrapper(['/']) });
|
||||||
|
|
||||||
|
act(() => result.current.list.setPage(2));
|
||||||
|
expect(result.current.params.get('page')).toBe('2');
|
||||||
|
act(() => result.current.list.setPage(1));
|
||||||
|
expect(result.current.params.get('page')).toBeNull(); // default omitted
|
||||||
|
});
|
||||||
|
|
||||||
|
it('filter changes reset page to 1', () => {
|
||||||
|
function Hookrunner() {
|
||||||
|
const [params] = useSearchParams();
|
||||||
|
const list = useListParams();
|
||||||
|
return { params, list };
|
||||||
|
}
|
||||||
|
const { result } = renderHook(() => Hookrunner(), { wrapper: wrapper(['/?page=5']) });
|
||||||
|
expect(result.current.params.get('page')).toBe('5');
|
||||||
|
act(() => result.current.list.setFilter('status', 'active'));
|
||||||
|
// page key removed because the setter resets pagination on filter change
|
||||||
|
expect(result.current.params.get('page')).toBeNull();
|
||||||
|
expect(result.current.params.get('filter[status]')).toBe('active');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('resetParams clears every search param', () => {
|
||||||
|
function Hookrunner() {
|
||||||
|
const [params] = useSearchParams();
|
||||||
|
const list = useListParams();
|
||||||
|
return { params, list };
|
||||||
|
}
|
||||||
|
const { result } = renderHook(() => Hookrunner(), {
|
||||||
|
wrapper: wrapper(['/?page=2&filter[status]=active']),
|
||||||
|
});
|
||||||
|
act(() => result.current.list.resetParams());
|
||||||
|
expect(Array.from(result.current.params.keys())).toEqual([]);
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -0,0 +1,110 @@
|
|||||||
|
// Bundle-8 / Audit M-010:
|
||||||
|
//
|
||||||
|
// Single hook for filter / sort / pagination state on every list page.
|
||||||
|
// Pre-Bundle-8, list pages stored these in local `useState` (see
|
||||||
|
// CertificatesPage:417 `const [page, setPage] = useState(1)`), which
|
||||||
|
// broke deep-linking and browser-back consistency. The DashboardPage
|
||||||
|
// already used `useSearchParams` directly; this hook canonicalises that
|
||||||
|
// pattern so the rest of the list pages can migrate mechanically.
|
||||||
|
//
|
||||||
|
// URL contract:
|
||||||
|
//
|
||||||
|
// ?page=2&page_size=25&sort=-created_at&filter[status]=active&filter[team_id]=t-platform
|
||||||
|
//
|
||||||
|
// Defaults are applied client-side — they do NOT appear in the URL when
|
||||||
|
// the user hasn't customised them, keeping shareable URLs short.
|
||||||
|
//
|
||||||
|
// Bundle-8 ships the hook + 1 demonstration migration (CertificatesPage)
|
||||||
|
// per the bundle prompt. The remaining list pages (IssuersPage,
|
||||||
|
// TargetsPage, AgentsPage, PoliciesPage, ProfilesPage, OwnersPage,
|
||||||
|
// TeamsPage, AgentGroupsPage, AuditEventsPage, NotificationsPage,
|
||||||
|
// JobsPage, RenewalPoliciesPage, DiscoveryPage) are deferred to a
|
||||||
|
// follow-up bundle — tracked as new ID `M-029`.
|
||||||
|
|
||||||
|
import { useCallback, useMemo } from 'react';
|
||||||
|
import { useSearchParams } from 'react-router-dom';
|
||||||
|
|
||||||
|
export interface ListParams {
|
||||||
|
/** Current page (1-indexed). Default: 1. */
|
||||||
|
page: number;
|
||||||
|
/** Page size. Default: 25. */
|
||||||
|
pageSize: number;
|
||||||
|
/** Sort key (e.g., `created_at`, `-name` for descending). Default: `''` (no sort). */
|
||||||
|
sort: string;
|
||||||
|
/** Filter map keyed by filter name (e.g. `{status: 'active', team_id: 't-platform'}`). */
|
||||||
|
filters: Record<string, string>;
|
||||||
|
}
|
||||||
|
|
||||||
|
export interface ListParamsControls {
|
||||||
|
params: ListParams;
|
||||||
|
setPage: (page: number) => void;
|
||||||
|
setPageSize: (pageSize: number) => void;
|
||||||
|
setSort: (sort: string) => void;
|
||||||
|
setFilter: (key: string, value: string | null) => void;
|
||||||
|
resetParams: () => void;
|
||||||
|
}
|
||||||
|
|
||||||
|
const DEFAULT_PAGE = 1;
|
||||||
|
const DEFAULT_PAGE_SIZE = 25;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Read filter/sort/pagination state from URL search params, with helpers to
|
||||||
|
* update the URL via `setSearchParams({ replace: true })` (preserves
|
||||||
|
* browser-back history without flooding it with intermediate states).
|
||||||
|
*
|
||||||
|
* @param defaults - per-page overrides for the global defaults above
|
||||||
|
*/
|
||||||
|
export function useListParams(defaults?: Partial<ListParams>): ListParamsControls {
|
||||||
|
const [searchParams, setSearchParams] = useSearchParams();
|
||||||
|
|
||||||
|
const params = useMemo<ListParams>(() => {
|
||||||
|
const page = parsePositiveInt(searchParams.get('page'), defaults?.page ?? DEFAULT_PAGE);
|
||||||
|
const pageSize = parsePositiveInt(
|
||||||
|
searchParams.get('page_size'),
|
||||||
|
defaults?.pageSize ?? DEFAULT_PAGE_SIZE,
|
||||||
|
);
|
||||||
|
const sort = searchParams.get('sort') ?? defaults?.sort ?? '';
|
||||||
|
const filters: Record<string, string> = { ...(defaults?.filters ?? {}) };
|
||||||
|
searchParams.forEach((value, key) => {
|
||||||
|
const m = /^filter\[(.+)\]$/.exec(key);
|
||||||
|
if (m && value) {
|
||||||
|
filters[m[1]] = value;
|
||||||
|
}
|
||||||
|
});
|
||||||
|
return { page, pageSize, sort, filters };
|
||||||
|
}, [searchParams, defaults]);
|
||||||
|
|
||||||
|
const updateParam = useCallback(
|
||||||
|
(key: string, value: string | null) => {
|
||||||
|
const next = new URLSearchParams(searchParams);
|
||||||
|
if (value === null || value === '') {
|
||||||
|
next.delete(key);
|
||||||
|
} else {
|
||||||
|
next.set(key, value);
|
||||||
|
}
|
||||||
|
// Bundle-8: filter / sort changes reset page to 1 (the existing
|
||||||
|
// CertificatesPage behaviour we're preserving). Only the page
|
||||||
|
// setter is allowed to set page > 1 directly.
|
||||||
|
if (key !== 'page') {
|
||||||
|
next.delete('page');
|
||||||
|
}
|
||||||
|
setSearchParams(next, { replace: true });
|
||||||
|
},
|
||||||
|
[searchParams, setSearchParams],
|
||||||
|
);
|
||||||
|
|
||||||
|
return {
|
||||||
|
params,
|
||||||
|
setPage: (page) => updateParam('page', page > 1 ? String(page) : null),
|
||||||
|
setPageSize: (size) => updateParam('page_size', size !== DEFAULT_PAGE_SIZE ? String(size) : null),
|
||||||
|
setSort: (sort) => updateParam('sort', sort || null),
|
||||||
|
setFilter: (key, value) => updateParam(`filter[${key}]`, value),
|
||||||
|
resetParams: () => setSearchParams(new URLSearchParams(), { replace: true }),
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
function parsePositiveInt(raw: string | null, fallback: number): number {
|
||||||
|
if (!raw) return fallback;
|
||||||
|
const n = Number(raw);
|
||||||
|
return Number.isFinite(n) && n > 0 ? Math.floor(n) : fallback;
|
||||||
|
}
|
||||||
@@ -0,0 +1,83 @@
|
|||||||
|
// Bundle-8 / Audit M-009:
|
||||||
|
// regression coverage for useTrackedMutation. Confirms that:
|
||||||
|
// 1. successful mutation invalidates each declared query key
|
||||||
|
// 2. caller's onSuccess fires after invalidation
|
||||||
|
// 3. 'noop' invalidates option requires noopReason at the type level
|
||||||
|
// (compile-time assertion via the discriminated union — runtime
|
||||||
|
// coverage here just confirms 'noop' passes through silently)
|
||||||
|
|
||||||
|
import { describe, it, expect, vi } from 'vitest';
|
||||||
|
import { renderHook, waitFor } from '@testing-library/react';
|
||||||
|
import { QueryClient, QueryClientProvider } from '@tanstack/react-query';
|
||||||
|
import { useTrackedMutation } from './useTrackedMutation';
|
||||||
|
import type { ReactNode } from 'react';
|
||||||
|
|
||||||
|
function withQueryClient(client: QueryClient) {
|
||||||
|
return ({ children }: { children: ReactNode }) => (
|
||||||
|
<QueryClientProvider client={client}>{children}</QueryClientProvider>
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
describe('useTrackedMutation — Bundle-8 / M-009', () => {
|
||||||
|
it('invalidates declared query keys on successful mutation', async () => {
|
||||||
|
const client = new QueryClient();
|
||||||
|
const invalidateSpy = vi.spyOn(client, 'invalidateQueries');
|
||||||
|
|
||||||
|
const { result } = renderHook(
|
||||||
|
() =>
|
||||||
|
useTrackedMutation({
|
||||||
|
mutationFn: async () => 'ok',
|
||||||
|
invalidates: [['certificates'], ['certificate', 'mc-001']],
|
||||||
|
}),
|
||||||
|
{ wrapper: withQueryClient(client) },
|
||||||
|
);
|
||||||
|
|
||||||
|
result.current.mutate(undefined);
|
||||||
|
await waitFor(() => expect(result.current.isSuccess).toBe(true));
|
||||||
|
|
||||||
|
// Once per declared key
|
||||||
|
expect(invalidateSpy).toHaveBeenCalledTimes(2);
|
||||||
|
expect(invalidateSpy).toHaveBeenCalledWith({ queryKey: ['certificates'] });
|
||||||
|
expect(invalidateSpy).toHaveBeenCalledWith({ queryKey: ['certificate', 'mc-001'] });
|
||||||
|
});
|
||||||
|
|
||||||
|
it('fires caller onSuccess after invalidation', async () => {
|
||||||
|
const client = new QueryClient();
|
||||||
|
const onSuccess = vi.fn();
|
||||||
|
const { result } = renderHook(
|
||||||
|
() =>
|
||||||
|
useTrackedMutation({
|
||||||
|
mutationFn: async () => 42,
|
||||||
|
invalidates: [['certificates']],
|
||||||
|
onSuccess,
|
||||||
|
}),
|
||||||
|
{ wrapper: withQueryClient(client) },
|
||||||
|
);
|
||||||
|
|
||||||
|
result.current.mutate(undefined);
|
||||||
|
await waitFor(() => expect(result.current.isSuccess).toBe(true));
|
||||||
|
expect(onSuccess).toHaveBeenCalledOnce();
|
||||||
|
expect(onSuccess.mock.calls[0][0]).toBe(42);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("noop variant doesn't invalidate but still runs caller onSuccess", async () => {
|
||||||
|
const client = new QueryClient();
|
||||||
|
const invalidateSpy = vi.spyOn(client, 'invalidateQueries');
|
||||||
|
const onSuccess = vi.fn();
|
||||||
|
const { result } = renderHook(
|
||||||
|
() =>
|
||||||
|
useTrackedMutation({
|
||||||
|
mutationFn: async () => 'noop-data',
|
||||||
|
invalidates: 'noop',
|
||||||
|
noopReason: 'fire-and-forget agent ping; no client cache impact',
|
||||||
|
onSuccess,
|
||||||
|
}),
|
||||||
|
{ wrapper: withQueryClient(client) },
|
||||||
|
);
|
||||||
|
|
||||||
|
result.current.mutate(undefined);
|
||||||
|
await waitFor(() => expect(result.current.isSuccess).toBe(true));
|
||||||
|
expect(invalidateSpy).not.toHaveBeenCalled();
|
||||||
|
expect(onSuccess).toHaveBeenCalledOnce();
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -0,0 +1,90 @@
|
|||||||
|
// Bundle-8 / Audit M-009:
|
||||||
|
//
|
||||||
|
// Thin wrapper around `useMutation` that REQUIRES the caller to declare
|
||||||
|
// what the mutation invalidates. Pre-Bundle-8, mutation invalidation
|
||||||
|
// was per-mutation discretion: a careless `useMutation` could leave
|
||||||
|
// stale data on screen for users who mutated and then queried.
|
||||||
|
//
|
||||||
|
// At Bundle-8 time the codebase has 56 useMutation sites and 70
|
||||||
|
// invalidateQueries calls — most mutations DO invalidate, but the
|
||||||
|
// pairing isn't enforced anywhere. This wrapper lets new mutations
|
||||||
|
// opt into the contract incrementally; the CI grep guard at
|
||||||
|
// `.github/workflows/ci.yml` ("Bundle-8 / M-009 mutation invalidation
|
||||||
|
// contract guard") flags new bare `useMutation` calls without a
|
||||||
|
// nearby invalidation marker.
|
||||||
|
//
|
||||||
|
// Usage:
|
||||||
|
//
|
||||||
|
// const renew = useTrackedMutation({
|
||||||
|
// mutationFn: () => renewCertificate(certId),
|
||||||
|
// invalidates: [['certificates'], ['certificate', certId]],
|
||||||
|
// });
|
||||||
|
//
|
||||||
|
// // Or, document why no invalidation is needed:
|
||||||
|
// const ping = useTrackedMutation({
|
||||||
|
// mutationFn: () => pingAgent(agentId),
|
||||||
|
// invalidates: 'noop', // server-side write that doesn't change cached data
|
||||||
|
// noopReason: 'agent ping records timestamp only — no client-side cache impact',
|
||||||
|
// });
|
||||||
|
|
||||||
|
import {
|
||||||
|
useMutation,
|
||||||
|
useQueryClient,
|
||||||
|
type QueryKey,
|
||||||
|
type UseMutationOptions,
|
||||||
|
} from '@tanstack/react-query';
|
||||||
|
|
||||||
|
interface TrackedMutationBase<TData, TError, TVariables, TContext>
|
||||||
|
extends Omit<UseMutationOptions<TData, TError, TVariables, TContext>, 'onSuccess'> {
|
||||||
|
/** Caller's onSuccess. The wrapper invalidates BEFORE invoking this. */
|
||||||
|
onSuccess?: UseMutationOptions<TData, TError, TVariables, TContext>['onSuccess'];
|
||||||
|
}
|
||||||
|
|
||||||
|
interface TrackedMutationWithInvalidates<TData, TError, TVariables, TContext>
|
||||||
|
extends TrackedMutationBase<TData, TError, TVariables, TContext> {
|
||||||
|
/** QueryKeys that should be invalidated on successful mutation. */
|
||||||
|
invalidates: QueryKey[];
|
||||||
|
noopReason?: never;
|
||||||
|
}
|
||||||
|
|
||||||
|
interface TrackedMutationNoop<TData, TError, TVariables, TContext>
|
||||||
|
extends TrackedMutationBase<TData, TError, TVariables, TContext> {
|
||||||
|
/** Explicitly opt out of invalidation. Requires a documented reason. */
|
||||||
|
invalidates: 'noop';
|
||||||
|
noopReason: string;
|
||||||
|
}
|
||||||
|
|
||||||
|
export type TrackedMutationOptions<TData, TError, TVariables, TContext> =
|
||||||
|
| TrackedMutationWithInvalidates<TData, TError, TVariables, TContext>
|
||||||
|
| TrackedMutationNoop<TData, TError, TVariables, TContext>;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Bundle-8 / M-009 — `useMutation` wrapper that enforces the
|
||||||
|
* invalidation contract. See file header for usage.
|
||||||
|
*/
|
||||||
|
export function useTrackedMutation<
|
||||||
|
TData = unknown,
|
||||||
|
TError = Error,
|
||||||
|
TVariables = void,
|
||||||
|
TContext = unknown,
|
||||||
|
>(options: TrackedMutationOptions<TData, TError, TVariables, TContext>) {
|
||||||
|
const queryClient = useQueryClient();
|
||||||
|
const { invalidates, onSuccess, ...rest } = options;
|
||||||
|
|
||||||
|
return useMutation<TData, TError, TVariables, TContext>({
|
||||||
|
...rest,
|
||||||
|
onSuccess: (data, variables, onMutateResult, context) => {
|
||||||
|
if (Array.isArray(invalidates)) {
|
||||||
|
for (const key of invalidates) {
|
||||||
|
// void-ignore: invalidateQueries returns a Promise but the
|
||||||
|
// wrapper deliberately fires-and-forgets — react-query will
|
||||||
|
// refetch in the background while React renders the next state.
|
||||||
|
void queryClient.invalidateQueries({ queryKey: key });
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if (onSuccess) {
|
||||||
|
return onSuccess(data, variables, onMutateResult, context);
|
||||||
|
}
|
||||||
|
},
|
||||||
|
});
|
||||||
|
}
|
||||||
@@ -0,0 +1,28 @@
|
|||||||
|
// Bundle-8 / Audit L-019 / CWE-79:
|
||||||
|
// the safeHtml.ts placeholder MUST throw if invoked before the real
|
||||||
|
// DOMPurify-backed implementation is wired. This catches the
|
||||||
|
// "imported the helper but forgot to add dompurify" regression at test
|
||||||
|
// time instead of at runtime against unsanitized HTML.
|
||||||
|
|
||||||
|
import { describe, it, expect } from 'vitest';
|
||||||
|
import { sanitizeHtml } from './safeHtml';
|
||||||
|
|
||||||
|
describe('safeHtml.sanitizeHtml — Bundle-8 / L-019', () => {
|
||||||
|
it('returns empty string for empty input without throwing', () => {
|
||||||
|
expect(sanitizeHtml('')).toBe('');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('throws a clear error for any non-empty input (placeholder behaviour)', () => {
|
||||||
|
expect(() => sanitizeHtml('<b>bold</b>')).toThrow(/safeHtml.sanitizeHtml is a placeholder/);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('error message points readers at the activation procedure', () => {
|
||||||
|
try {
|
||||||
|
sanitizeHtml('<script>x</script>');
|
||||||
|
throw new Error('should have thrown');
|
||||||
|
} catch (e) {
|
||||||
|
expect(String(e)).toMatch(/dompurify/);
|
||||||
|
expect(String(e)).toMatch(/safeHtml\.ts file header/);
|
||||||
|
}
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -0,0 +1,54 @@
|
|||||||
|
// Bundle-8 / Audit L-019 / CWE-79 (XSS):
|
||||||
|
//
|
||||||
|
// Single chokepoint for any code that needs `dangerouslySetInnerHTML`.
|
||||||
|
// At Bundle-8 time the codebase has ZERO `dangerouslySetInnerHTML` sites
|
||||||
|
// (verified via `grep -rn dangerouslySetInnerHTML web/src/`); this file
|
||||||
|
// is preventive — when a future feature genuinely needs to render a
|
||||||
|
// trusted-but-rich HTML fragment (markdown email body, signed
|
||||||
|
// notification template, etc.) it MUST route through `sanitizeHtml`
|
||||||
|
// instead of inlining the dangerous attribute.
|
||||||
|
//
|
||||||
|
// The CI regression guard at `.github/workflows/ci.yml` blocks any new
|
||||||
|
// bare `dangerouslySetInnerHTML` from landing — see the
|
||||||
|
// "Bundle-8 / L-019 dangerouslySetInnerHTML guard" step.
|
||||||
|
//
|
||||||
|
// We don't take a runtime DOMPurify dependency in Bundle-8 because the
|
||||||
|
// allowlist is empty at HEAD. When the first real call site lands, add
|
||||||
|
// `dompurify` to package.json and replace the body of `sanitizeHtml`
|
||||||
|
// with a real DOMPurify call. Until then this file documents the
|
||||||
|
// contract and provides a typed boundary the linter can recognise.
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Sanitize an arbitrary HTML string before passing it to React's
|
||||||
|
* `dangerouslySetInnerHTML` prop. Bundle-8 placeholder — see the file
|
||||||
|
* doc comment above for the activation procedure when the first real
|
||||||
|
* call site lands.
|
||||||
|
*
|
||||||
|
* @param html - the untrusted HTML payload
|
||||||
|
* @param _options - reserved for the future DOMPurify config object
|
||||||
|
* @returns the sanitized string ready to assign to `__html`
|
||||||
|
*/
|
||||||
|
export function sanitizeHtml(html: string, _options?: SanitizeOptions): string {
|
||||||
|
if (!html) return '';
|
||||||
|
// Bundle-8: until the first real call site lands, refuse to render
|
||||||
|
// anything. Throwing here is the safe default — a future regression
|
||||||
|
// that imports this helper without enabling DOMPurify will fail loud
|
||||||
|
// at runtime (test) instead of silently rendering attacker HTML.
|
||||||
|
throw new Error(
|
||||||
|
'safeHtml.sanitizeHtml is a placeholder. Add dompurify to package.json ' +
|
||||||
|
'and implement the body before using this helper. See ' +
|
||||||
|
'web/src/utils/safeHtml.ts file header for the contract.',
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Reserved for the DOMPurify configuration object that the future real
|
||||||
|
* implementation will accept. Documented now so call-site signatures
|
||||||
|
* don't change when the body lights up.
|
||||||
|
*/
|
||||||
|
export interface SanitizeOptions {
|
||||||
|
/** Tags that should survive sanitization (default: `['b','i','em','strong','a','p','br']`) */
|
||||||
|
allowedTags?: string[];
|
||||||
|
/** Attributes that should survive sanitization (default: `['href','title']`) */
|
||||||
|
allowedAttr?: string[];
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user