mirror of
https://github.com/shankar0123/certctl.git
synced 2026-06-07 17:12:04 +00:00
fix(bundle-8): Frontend Hardening — 2 audit findings closed + 3 partial
Closes Audit-2026-04-25 L-015 (Low) and L-019 (Low) — both
verified-already-clean at HEAD; new CI regression guards prevent
regression. Partial closures for M-009, M-010, M-026 — Bundle 8 ships
the helpers + contract tests + a soft CI budget guard, defers the
long-tail per-page migrations to a new tracker ID M-029.
What changed
- web/src/utils/safeHtml.ts (NEW) — sanitizeHtml() chokepoint for
any future code that genuinely needs dangerouslySetInnerHTML.
Bundle-8 placeholder body throws — DOMPurify dependency is the
activation procedure documented in the file header.
- web/src/components/ExternalLink.tsx (NEW) — single chokepoint for
target="_blank" anchors. Hardcodes rel="noopener noreferrer".
- web/src/hooks/useListParams.ts (NEW) — URL-state hook for filter /
sort / pagination state on list pages. Canonicalises the existing
DashboardPage useSearchParams pattern. Per-page migrations of the
~14 remaining list pages tracked as M-029.
- web/src/hooks/useTrackedMutation.ts (NEW) — useMutation wrapper
enforcing the M-009 invalidation contract via discriminated-union
type: caller MUST declare invalidates: QueryKey[] OR
invalidates: 'noop' + noopReason: string.
- 4 new Vitest test files — full unit coverage for ExternalLink
(target/rel preservation), safeHtml (placeholder throws + activation
hint), useListParams (URL contract / defaults / filter-resets-page),
useTrackedMutation (invalidate-then-onSuccess / noop variant).
- .github/workflows/ci.yml — three new regression guards:
Bundle-8 / L-015: greps for any target="_blank" outside ExternalLink
that lacks rel="noopener noreferrer"; clean at HEAD.
Bundle-8 / L-019: greps for any dangerouslySetInnerHTML outside
safeHtml.ts; clean at HEAD (0 sites).
Bundle-8 / M-009: SOFT budget guard — useMutation sites must not
exceed invalidation sites + 5. At HEAD: 61 mutations vs 82
invalidations + 5 = 87 budget. Stricter per-site enforcement
tracked as M-029.
Verification at HEAD
- web/src/ target=_blank sites: 3 (all in OnboardingWizard.tsx)
— all three already carry rel="noopener noreferrer". L-015 closed.
- web/src/ dangerouslySetInnerHTML sites: 0. L-019 closed.
- useMutation sites: 61 / invalidateQueries: 82 (M-009 budget healthy)
Per-finding mapping
- L-015 closed (CWE-1022) — verified-already-clean + ExternalLink
component + CI grep guard.
- L-019 closed (CWE-79) — verified-already-clean + safeHtml chokepoint
+ CI grep guard.
- M-009 partial — useTrackedMutation wrapper authored; soft CI budget
guard. Migrating the 56 existing useMutation sites to the wrapper
tracked as M-029.
- M-010 partial — useListParams hook authored + tested. Per-page
migration of the ~14 list pages tracked as M-029.
- M-026 partial — bundle-prompt called for XSS-hardening tests on the
T-1 deferred allowlist of 14 pages. Bundle 8 ships the testing
pattern via the new helpers but does NOT execute the per-page
migrations — tracked as M-029.
NOT addressed in this bundle (deferred to M-029)
- Migrating existing 56 useMutation sites to useTrackedMutation
- Migrating ~14 list pages from local useState to useListParams
- Adding XSS-hardening tests to the 14 T-1-deferred pages
Verification
- npx tsc --noEmit → clean
- npx vitest run on the 4 new Bundle-8 test files → 15/15 pass
- L-015 grep guard simulation → clean
- L-019 grep guard simulation → clean
- M-009 budget simulation → 61 ≤ 87 (clean)
- go vet ./... → clean (no backend changes)
- python3 yaml.safe_load(api/openapi.yaml) → clean
- python3 yaml.safe_load(.github/workflows/ci.yml) → clean
Backwards compatibility
- All 4 new helper files are additive; no existing call sites were
modified. Existing list pages keep their useState pagination until
M-029 ships per-page migrations.
Bundle 8 of the 2026-04-25 comprehensive audit. Per-page migration
backlog tracked as new audit finding M-029.
This commit is contained in:
@@ -825,6 +825,77 @@ jobs:
|
||||
ALLOWLIST_SIZE=$(echo "$ALLOW" | tr '|' '\n' | wc -l)
|
||||
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)
|
||||
# G-3 master closed cat-g-163dae19bc59 (docs-only env vars
|
||||
# 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