feat: system config feature flags and rate-limit exemption admin UI#465
feat: system config feature flags and rate-limit exemption admin UI#465brandonrc wants to merge 1 commit into
Conversation
Add a SystemConfigProvider that fetches GET /api/v1/system/config and exposes derived feature flags through useSystemConfig/useFeatureFlags. Gate the scanner-dependent sidebar entries (Scan Results, DT Projects) on the reported scanner flags, and surface the configured max upload size in the artifact upload dropzone with a client-side oversize guard. Add a Rate Limits admin page that shows the effective per-window limits and lets admins view, add, and remove rate-limit exemptions for usernames, service accounts, and CIDR ranges. The page degrades gracefully when the backend has not shipped the exemption-management endpoints. Includes unit tests for the new API clients and provider, updates to the sidebar tests for flag-driven gating, and Playwright e2e specs covering feature-flag gating and the exemption admin flow. Closes #271 Closes #270
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
brandonrc
left a comment
brandonrc
left a comment
There was a problem hiding this comment.
Accessibility review (WCAG 2.2 AA) of the rate-limit admin page (table, add/remove dialogs) and feature-flag-gated nav.
Positive findings
- Add Exemption dialog: all three controls are labelled.
<Label htmlFor="exemption-type">+SelectTrigger id,<Label htmlFor="exemption-value">(label text is dynamic viameta.label, so the field name tracks the selected type),<Label htmlFor="exemption-note">. Good (4.1.2, 3.3.2). - Remove flow uses
AlertDialog(Radix) withAlertDialogTitle/AlertDialogDescription: proper modal alertdialog with focus trap and Escape. Good (2.4.3, 4.1.2). - Per-row remove button has
aria-label={Remove exemption ${ex.value}}(icon-only button with an accessible name) plustitle; the env-sourced disabled state is native (4.1.2, 4.1.1). - ConfigCard uses a real
<dl>/<dt>/<dd>for read-only config (1.3.1). Nice. - Feature-flag nav gating filters items from the array before render, so hidden entries are removed from the DOM (not just visually hidden) and are unreachable by keyboard/SR. Correct approach, no phantom focus targets.
Gaps
-
Serious (3.3.1 / 4.1.3): the Add Exemption form reports validation only via
toast.error(validateExemption(...)). The error is transient and not tied to the offending field. On invalid input,#exemption-valueshould setaria-invalidandaria-describedbyto a persistentrole="alert"message inside the dialog, rather than relying on a toast that can render outside the dialog and is not associated with the control. Same recurring pattern as the rest of the v1.2.0 batch (see #459's create-dialog pattern). -
Moderate (1.3.1): the exemptions table has an empty action header
<TableHead className="w-[1%]" />. Add<span className="sr-only">Actions</span>so the column is named for SR. Also addaria-label="Rate-limit exemptions"on the<Table>to match the named-table pattern used in #462. -
Minor (1.4.1): Source is conveyed by Badge variant (Environment vs Manual), but the text labels carry the meaning, so this is fine.
-
Minor: the "—" empty-note placeholder is read as "em dash" by SR; low priority, consider sr-only "no note".
No hard blocker. Item 1 (dialog form errors not programmatically associated) is the one to fix for consistency. Review-only, not approving.
brandonrc
left a comment
brandonrc
left a comment
There was a problem hiding this comment.
Security/API review (CIDR validation + sensitive-config exposure lens).
Net: safe. Admin surface is gated, the public config endpoint exposes only booleans/limits, and client CIDR validation is advisory with the backend as the authority.
Authorization — OK. The page lives under src/app/(app)/(admin)/rate-limits/page.tsx, and the (admin) route group is wrapped by RequireAdmin in its layout, so non-admins cannot render it. The exemption endpoints are documented as admin-guarded server-side (/api/v1/admin/rate-limits[/exemptions]). Client gating is UX only; the real check must stay on the backend (it is, per the admin router) — confirm that holds, since a non-admin could otherwise call these directly.
isValidCidr / validateExemption — ADVISORY, ACCEPTABLE. The comment correctly states the backend does authoritative validation. The client check is a reasonable sanity filter: IPv4 requires 4 octets 0-255 and prefix <=32; IPv6 requires prefix <=128 and a [0-9a-fA-F:] charset. It is intentionally loose (e.g. it would accept malformed IPv6 like :::/64 or out-of-range-looking forms), but since it only gates the submit button and the backend is authoritative, this is not a security issue — just do not rely on it as a trust boundary. No ReDoS: the regexes are linear, no catastrophic backtracking.
Sensitive config exposure — OK. systemConfigApi consumes GET /api/v1/system/config, which returns only non-sensitive booleans/strings (scanner/auth enable flags, search/storage backend names, upload limit, demo/guest flags, optional oidc_issuer). No secrets, connection strings, CIDRs, usernames, or token material are surfaced to non-admins. oidc_issuer is a public URL by design. zod .passthrough() with explicit field mapping means a future backend field that accidentally carries something sensitive would still need the web app to model+render it, so no silent leak through this client. Defaults are permissive for nav visibility but reveal nothing.
Note (out of scope, confirm server-side): ensure /api/v1/system/config truly returns only these flags and never echoes the raw env values (e.g. the actual RATE_LIMIT_TRUSTED_CIDRS list) to unauthenticated callers. The web type does not request them; just verify the backend response does not include them.
No request-changes.
brandonrc
left a comment
brandonrc
left a comment
There was a problem hiding this comment.
Test/E2E quality review (v1.2.0 batch). Verdict: system-config spec is genuinely strong; rate-limits spec leans on a skip and tolerant branches that can pass without testing the add/remove flow.
system-config (#271) — strong. public system config endpoint returns the expected shape asserts the full contract: typed checks on max_upload_size_bytes, demo_mode, guest_access_enabled, search_engine, storage_backend, plus nested scanners.* and auth.* booleans. No skip. The scanner nav entries match the backend scanner flags test is the best part: it reads the live config and asserts BOTH directions — if scanners on, the nav link toBeVisible; else toHaveCount(0). That two-sided assertion genuinely proves the gating logic rather than just "link exists." This distinguishes fixed-vs-broken regardless of how the CI backend is configured.
upload-size advertise test. Has two skips (no limit advertised; upload tab unavailable) then asserts getByText(/max\s/i). The /max\s/i matcher is loose — it would match almost any "max ..." string on the page, not necessarily the configured size. Weak assertion behind two skips; low value but not harmful.
rate-limits (#270) — the soft spots.
add then remove a username exemptionis the core round-trip, and it's gated: if the new row isn't visible in 8s,test.skip(true, 'Backend does not expose rate-limit exemption management'). So on any backend without the endpoint (or if creation is slow), the entire add/remove/confirm flow never asserts. The remove path (toHaveCount(0)) only runs in the happy case. This is the load-bearing behavioral test and it can no-op.invalid CIDR is rejected client-sideis good and non-skippable: switches type to CIDR, fillsnot-a-valid-cidr, asserts a/valid cidr/itoast ANDdialog.toBeVisible()(stays open). Real negative case proving client validation. Keep this.- The dialog-fields and heading tests are presence smoke checks, fine as such.
no uncaught console errors on loadfilters favicon/hydrate/ResizeObserver — reasonable, but it's a guard, not coverage of the feature.
Recommendation. For #270 to have non-conditional coverage of the actual exemption management, either confirm the v1.2.0 backend ships the endpoints (then drop the skip so a broken add/remove fails), or mock the endpoint. As-is, the CIDR-validation and the system-config nav-gating tests are the genuine coverage; the exemption round-trip is conditional. Not requesting changes — system-config carries this and the CIDR test is real.
brandonrc
left a comment
brandonrc
left a comment
There was a problem hiding this comment.
Frontend/React correctness review. No blockers. The strongest PR in the batch: clean provider, zod at the trust boundary, good error/unavailable states, and complete tests.
Correctness notes
-
SystemConfigProviderplacement aboveAuthProvideris correct given the endpoint is public (/api/v1/system/config, documented "requires no authentication"). The query is unconditional withstaleTime: 10mandretry: false, which is appropriate for a public, rarely-changing config. Because it's public, there's no need to refetch on login. Good. -
Permissive-defaults design is sound.
DEFAULT_SYSTEM_CONFIGkeeps nav surfaces visible on fetch failure, with the deliberate scanner exception defaulting to hidden, and the rationale is documented. On error/loading the context still exposes a concreteSystemConfig, so consumers never null-check. The provider throws a clear error when used outside its scope, with a test. -
Sidebar gating (
visibleSecurityItems) keys off the real hrefs (/security/scans,/security/dt-projects, verified againstsecurityItems), gates Scan Results on Trivy-or-OpenSCAP and DT Projects on Dependency-Track, and leaves the rest of the Security group always visible. Matches the stated intent and is unit-tested both ways. -
file-upload.tsxclient-side size check is correct defense-in-depth: it rejects oversize files before upload (clears the input, sets an error, returns early) while the backend still enforces authoritatively.handleFiledeps correctly includemaxUploadSizeBytes. The advertised max in the dropzone copy only shows when > 0. -
Rate-limits page has proper admin guard, distinct loading/error/empty/unavailable states, a confirm dialog for the destructive remove, and env-sourced rows shown read-only (delete disabled with an explanatory title). Mutations invalidate the right query keys. This is the model the other admin pages (e.g. #462 routing rules) should follow for error UX.
-
API clients (
system-config.ts,rate-limits.ts) validate every response with zod.safeParse+.passthrough()for forward-compat, accept both array and{exemptions: [...]}list shapes, andencodeURIComponentthe delete id. TheapiFetchescape hatch is used as intended.
Nits
isValidCidr: an empty prefix like10.0.0.0/parses asNumber("") === 0, a valid integer >= 0, so it passes as/0. Backend validates authoritatively, but tightening to require a non-empty prefix string would catch the obvious typo client-side.rate(window)returns${limit} requestswhenwindow_secs <= 0; fine, just confirm the backend never sends a 0 window where a real window is expected.
LGTM.
* feat(repo-browser): add 'Invalidate cache' action to artifact details dialog (#446) Adds an Invalidate cache button in the artifact-details dialog that calls the new backend endpoint POST /api/v1/repositories/{key}/cache/invalidate ?path=... (companion artifact-keeper#1540 / artifact-keeper#1539). Behaviour: - Button is gated on repository.repo_type === 'remote', so the action is hidden for Local / Virtual / Staging repos that have no proxy cache. The backend also rejects with 400 there, defence in depth. - React-Query mutation invalidates ['artifacts', repoKey] and ['repository', repoKey] on success so subsequent fetches go back to upstream and the cached metadata refreshes. - Toast on success says next download will re-fetch from upstream so operators understand what happened. Errors surface via the existing mutationErrorToast helper, including the backend's user-message. - Pending state disables the button and shows 'Invalidating...' text to prevent double-fires. The API wrapper goes through apiFetch (src/lib/api/fetch.ts) because the generated SDK has not been regenerated against the new endpoint yet; once it has, the wrapper can collapse to the typed SDK call in a follow-up. Tests: - artifactsApi.invalidateCache: pins the URL shape (POST, /cache/invalidate, ?path=, credentials: include). - URL-encoding of both repo key and path so '+', '/', and spaces survive round-trip back to the backend. - Surfaces non-ok responses (e.g. 400 for non-remote repo, 503 for missing storage) as thrown errors. Closes #446 * feat(settings): show + edit proxy cache TTL on Remote repo Settings tab (#448) Adds a Proxy Cache section to the repo Settings tab that displays and edits the proxy cache TTL on Remote (proxy) repositories. Hidden for Local / Virtual / Staging repos -- those have no proxy cache and the backend rejects PUT /:key/cache-ttl on them anyway. The TTL endpoints (GET / PUT /api/v1/repositories/{key}/cache-ttl) already exist and the SDK already exports getCacheTtl / setCacheTtl, so this is a pure UI change. Behaviour: - Section is gated on repository.repo_type === 'remote'. - Initial value comes from GET /:key/cache-ttl, which falls back to DEFAULT_CACHE_TTL_SECS (86400 = 24h) when nothing is stored -- matching the contract documented by the docs PR #71. - Inline validation mirrors the backend's validate_cache_ttl range (1..=2_592_000). Out-of-range values get an aria-invalid input, an inline error, and disable the Save button. - Helper line shows the human-readable equivalent ('approximately 24 hours', '1 day 6 hours') so operators don't have to mentally convert seconds. - Plugs into the existing hasChanges / Save Changes / Discard workflow. Save dispatches the general-fields update and the new setCacheTtl in parallel via Promise.allSettled; failures on one side don't roll back the other, and each mutation has its own onError toast so the operator can tell which side failed. Adds repositoriesApi.getCacheTtl / setCacheTtl wrappers using the typed SDK calls. No hand-written apiFetch needed. Tests: - getCacheTtl happy path + error surfacing. - setCacheTtl happy path with body-shape pin (cache_ttl_seconds, NOT the legacy 'value' form that the docs PR #71 corrected). - setCacheTtl error surfacing including the backend's 'remote (proxy) repositories' rejection. Closes #448 * feat(repo-browser): show artifact cache cached_at + expires_at in details dialog (#449) Adds two relative-time rows to the artifact details dialog ('Cached' and 'Cache expires') for Remote (proxy) repositories, so operators reading the panel can decide between waiting for the natural refresh and clicking the 'Invalidate cache' button (#446 / #447) without leaving the UI. Companion backend work: artifact-keeper#1541 / #1542 exposes the cache_cached_at and cache_expires_at fields on GET /:key/artifacts/:path. This change is forward-compatible -- until the backend PR lands the rows simply don't appear, no errors. Behaviour: - Both rows are gated on repository.repo_type === 'remote' AND the value being present. Local / Virtual / Staging never show them; Remote repos without a cache-metadata blob (e.g. an artifact direct-uploaded but never proxy-fetched) also hide the rows. - Visible value is the relative-time string ('in 4 hours' / '12 minutes ago' / 'expired 3 days ago, will re-fetch on next download') for quick scanning; the hover-tooltip carries the full ISO-8601 timestamp. New helpers in src/lib/cache-time.ts (pure, testable): - formatRelativeTimestamp(iso, now?) -> 'in 4 hours' style. Uses Intl.RelativeTimeFormat for locale-aware output without a date-fns dep. Picks the largest unit that gives a magnitude >= 1 so output stays compact (we surface 'in 2 days' not 'in 48 hours'). - formatCacheExpiry(iso, now?) -> biased for the expires framing; past timestamps wrap as 'expired ..., will re-fetch on next download'. DetailRow gains an optional title prop so the visible value (relative time) can differ from the hover-tooltip (absolute timestamp). Artifact type gains optional cache_cached_at / cache_expires_at. The SDK doesn't carry these fields yet (regenerates from OpenAPI after the backend PR lands); until then adaptArtifact plumbs them via a narrowed runtime cast. Once the SDK regenerates, the cast can collapse. Tests: - src/lib/__tests__/cache-time.test.ts: 9 cases covering future / past direction, largest-unit selection, unparseable-timestamp fallback, and the 'expired ..., will re-fetch' framing including the boundary t == now case. - src/lib/api/__tests__/artifacts.test.ts: 3 new cases pinning adaptArtifact plumbing -- fields populated when present, undefined when omitted, undefined when explicitly null (defensive). Closes #449 * test(settings): add component tests for new Proxy Cache section Closes the new-code coverage gap that the CI gate flagged (56% on the initial commit; threshold is 80%). The previous commit added the UI section + API wrappers, but only the wrappers had unit-level coverage. This commit adds component-level tests against the React tree, pinning the contract the issue body promises: - Section is hidden for Local / Virtual / Staging repos; the GET is not even issued (gated on 'enabled: isRemote' in the useQuery). - Section is visible for Remote repos and renders the TTL fetched from getCacheTtl. - Editing the input plugs into the existing unsaved-changes bar. - Save invokes setCacheTtl with the parsed integer and shows the success toast; the general-fields update mutation does NOT fire when only the TTL changed (so we don't generate empty audit-log PATCH entries). - Out-of-range values (>2,592,000 and 0) show the inline error, flip aria-invalid on the input, and disable the Save button. - Discard reverts the TTL override to the fetched value. - setCacheTtl failures (e.g. 503 from a misconfigured proxy) surface the existing 'Failed to save cache TTL' error toast. Mock surface for repositoriesApi extended with getCacheTtl / setCacheTtl. No production code changes. * fix(tickets): bind download/stream ticket resource_path to the request path The backend ticket middleware compares the bound resource_path against request.uri().path() by byte equality at consume time, so the minter must send the exact absolute path the later request carries. createDownloadTicket sent `${repoKey}/${artifactPath}` and createStreamTicket sent `migration/${jobId}`. Both were non-absolute (rejected with "resource_path must start with '/'") and would not have byte-matched the real request path even if made absolute, so UI artifact downloads and migration progress streaming failed. Bind to the actual request paths: - /api/v1/repositories/{repoKey}/download/{artifactPath} - /api/v1/migrations/{jobId}/stream Add regression tests asserting the resource_path argument for both call sites. Fixes #453 * fix: absolute download URL and resilient server version display Resolves two v1.2.0 web bugs. #455: The artifact detail "Download URL" field showed a host-less path, so copying it produced a broken URL. Add artifactsApi.getAbsoluteDownloadUrl, which resolves the path against NEXT_PUBLIC_API_URL or the current window origin, and use it for the copyable detail field. #456: The sidebar hid the backend version whenever /health returned a non-2xx status, even though the version is present in the response body. adminApi.getHealth now detects the HealthResponse shape on the SDK error path (the backend returns 503 with a full body when degraded) and adapts it, so the version stays visible. Adds unit tests for both helpers and a Playwright spec under e2e/suites/interactions/repositories verifying the copied Download URL is absolute and well-formed and that the sidebar shows the backend version. * feat(maven): search by GAV and surface POM in the UI Adds Maven GAV-aware search and POM access for Maven repositories. Search (#441): the GAVC tab previously sent groupId/artifactId/version as the backend's path/name/version params, which advanced-search ignores (it matches a single full-text query over name + path + version). The tab now folds the GAV, classifier, and a new Extension field into one query string scoped to the maven format, so coordinate searches return results. A shared buildMavenSearchQuery helper centralizes the term assembly. POM and GAV (#442): the grouped Maven browser now renders each component file as a download link (path derived from the GAV layout) and tags the .pom file with a POM badge. The artifact detail view gains a Maven section showing parsed groupId/artifactId/version and a copy/paste pom.xml dependency snippet. New src/lib/maven.ts holds the pure coordinate helpers (query building, path derivation, POM detection, GAV parsing, snippet rendering) with unit tests, plus a Playwright spec covering GAV search and POM reachability. * fix(a11y): repo dialog and SSO form WCAG 2.2 AA gaps Address four accessibility gaps in the repository create/edit dialogs and SSO settings forms: - #410: the repo edit dialog now mirrors the upstream-auth save outcome into an in-dialog aria-live region (role="status" on success, role="alert" with aria-live="assertive" on error) so screen-reader users hear the result. The parent mutation previously surfaced only a visual toast. - #411: the create-dialog duplicate-key error now has role="alert", and the key input sets aria-invalid and aria-describedby pointing at the message. - #412: toggling the upstream-auth view to edit (and back) now moves focus to the first control of the newly-revealed view instead of dropping focus on the body. - #413: required inputs across the repo dialogs and SSO OIDC/LDAP/SAML forms expose aria-required; the edit-dialog auth-type select gains a programmatic label; the repo row actions menu trigger gains an accessible name; the edit-key warning is associated via aria-describedby. Adds a Playwright spec under e2e/suites/interactions/repositories asserting the error/alert association, focus movement on toggle, and live-region save announcement. * fix(repositories): pagination, file click-through, and full file listing in Maven grouped view Grouped (Maven/Gradle) mode in the repository browser had three defects: - #443: the grouped view rendered no pagination, so only the first 20 GAV components were ever shown. Extracted the DataTable pagination markup into a shared DataTablePagination component and wired it into MavenComponentList, driven by the same page/pageSize state the flat list uses. - #444: file rows inside an expanded GAV group were plain text and did nothing on click. They are now buttons that reconstruct the Maven path (groupId/artifactId/version/filename) and open the artifact detail dialog, matching flat-mode behaviour. - #445: non-jar files such as .zip and checksum sidecars were not openable in grouped mode. The grouped response already carries every filename per GAV; surfacing each as a clickable row now exposes them with full metadata via the detail dialog. Also fixed artifactsApi.get to encode path segments individually so the backend wildcard route (/:key/artifacts/*path) matches reconstructed paths instead of 404ing on percent-encoded slashes. Closes #443 Closes #444 Closes #445 * test(e2e): verify remote-repo cached artifacts show in the browser (#424) Adds a Playwright spec covering the v1.2.0 regression where packages pulled through a remote (proxy) repository filled up storage but never appeared in the repo browser, so they couldn't be browsed or scanned. The spec pulls a small package through the seeded `e2e-npm-remote` proxy, then asserts the cached entry appears both in the listing API (GET /api/v1/repositories/{key}/artifacts) and in the Artifacts tab of the repo detail page. It skips gracefully if the upstream registry is unavailable, since upstream availability is not what's under test. The fix is backend-only (artifact-keeper#1567 / #1548): the listing for remote repos is now reconstructed from the proxy cache. The web client already consumed the corrected data through the same endpoint, so no web code change is required. Closes #424 * feat(search): surface OpenSearch capabilities in the search UI The backend migrated search indexing from Meilisearch to OpenSearch in 1.2.0. This wires the search UI to the capabilities that migration exposes and fixes a health-card regression caused by the renamed health check. Search results: - Add relevance as the default sort (sends no sort_by so the backend applies its own ranking) plus a Downloads sort option. - Add a sort-direction toggle that sends sort_order=asc|desc; it is disabled while sorting by relevance, which has no direction. - Drop the client-side re-sort that only reordered the current page and fought the server's ordering across paginated results. - Render the highlight snippets the backend returns, parsing the <em> markers into React nodes rather than injecting raw HTML. - Surface the formats and repositories facets as clickable refine chips that re-issue the search with the selected facet applied. System health: - Read the search-engine health check from checks.opensearch, falling back to the legacy checks.meilisearch field so the dashboard "Search Engine" card renders against both backend versions. Adds a Playwright spec covering the sort options, direction toggle, sort_by/sort_order request params, relevance default, and facet refine behavior, plus unit tests for the health adapter mapping. * feat(repositories): add release target and routing rules settings Add two repository Settings tab sections: Release target (#260): staging repositories can be linked to a local release repository of the same format. The picker lists eligible local repos and saves through PATCH /repositories/{key} with release_repository_key. An empty selection unlinks. Non-staging repos show a notice instead of the control. Routing rules (#263): view, add, edit, and remove path-rewriting rules for remote, virtual, and staging repositories. Each rule is a regex path_pattern and a rewrite_to template referencing capture groups. Rules are stored as a single ordered list via the routing-rules endpoints; removing the last rule clears the config. Both sections reuse the repositoriesApi wrapper, with new getRoutingRules, setRoutingRules, deleteRoutingRules, and setReleaseTarget methods backed by the shared apiFetch helper since the generated SDK does not expose these endpoints yet. Adds Playwright e2e specs covering the API contracts and the UI flows for both features. * Add package age policy and upload size limit configuration UIs Adds two repository configuration surfaces for v1.2.0: - Package age policy (#265): a Package Age Policy section on the repo Settings tab that holds freshly published packages in quarantine for a configurable cooldown window. Sends quarantine_enabled and quarantine_duration_minutes to PATCH /api/v1/repositories/{key}. - Upload size limit (#189): the admin Settings -> Storage tab now lets admins edit the max upload size (value + MB/GB unit), persisted via POST /api/v1/admin/settings. The repo Settings tab surfaces the effective limit read-only so repo owners see the ceiling that applies. Includes unit tests for the new helpers and components, plus Playwright e2e specs covering both configuration flows. * feat: system config feature flags and rate-limit exemption admin UI Add a SystemConfigProvider that fetches GET /api/v1/system/config and exposes derived feature flags through useSystemConfig/useFeatureFlags. Gate the scanner-dependent sidebar entries (Scan Results, DT Projects) on the reported scanner flags, and surface the configured max upload size in the artifact upload dropzone with a client-side oversize guard. Add a Rate Limits admin page that shows the effective per-window limits and lets admins view, add, and remove rate-limit exemptions for usernames, service accounts, and CIDR ranges. The page degrades gracefully when the backend has not shipped the exemption-management endpoints. Includes unit tests for the new API clients and provider, updates to the sidebar tests for flag-driven gating, and Playwright e2e specs covering feature-flag gating and the exemption admin flow. Closes #271 Closes #270 * fix(settings): reflect persisted max upload size in editor (review #464) The UploadSizeSetting useState initializer seeded from an undefined currentBytes while admin-settings was still loading, so the editor showed an empty 'No limit' even when a limit was configured and never refreshed once the query resolved. Sync local state during render when the persisted value changes, gated on !dirty so unsaved edits are never clobbered. * fix(repositories): a11y error association + write-through guards (review #462/#464) - Routing rules: associate the add-rule validation error with the pattern input via aria-invalid + aria-describedby on a persistent role=alert element, validate the regex inline, and gate the resync on !dirty so a window-focus refetch no longer clobbers unsaved edits. - Age policy: associate the cooldown error with the input via a persistent role=alert region, and disable Save until an explicit change so a pristine form cannot overwrite an existing policy. - Release target: disable Save until the selection changes, so a pristine picker cannot unlink an existing target. * fix(search,admin): aria-live result announcements + exemption a11y, harden e2e (review #463/#465/#464) - Search: add a visually-hidden aria-live status region that announces the result count, active sort, and active facets so sort/filter changes are not silent to screen readers. - Rate-limit exemption dialog: associate the validation error with the value input via aria-invalid + aria-describedby on a persistent role=alert element instead of a toast. - E2E hardening: the upload-size and age-policy save tests now assert the POST/PATCH actually fires and succeeds rather than passing vacuously; the rate-limit exemption add/remove round-trip now asserts instead of skipping. * chore: bump web version to 1.2.0 * test(search): expect facets + quarantine fields from advancedSearch (#463) PR #463 extended the advancedSearch adapter to surface OpenSearch facets and per-item quarantine fields but left the unit test's exhaustive toEqual stale, so it failed once merged. Update the expectation to match the adapter contract. * fix(repositories): cache-UI review fixes for TTL a11y, invalidate confirm, dead code Addresses code-review findings on the cache-UI PRs that landed on release/web-1.2.0 after the integration branch was built. repo-settings-tab.tsx (#450 proxy cache TTL): - Associate the TTL validation error with the input via aria-describedby pointing at a persistent role="alert" element (was a conditional, unassociated paragraph), mirroring the age-policy field pattern. - Disable the Discard button while a Save is in flight so it cannot race an in-flight mutation. repo-detail-content.tsx (#447 invalidate cache): - Wrap the destructive "Invalidate cache" action in an AlertDialog confirmation instead of firing immediately on click. - After a successful invalidate, close the details dialog so it does not show stale cache_cached_at / cache_expires_at fields. - Announce delete and invalidate outcomes through a dedicated polite live region in addition to the toast. - Add the missing trailing newline at EOF (eol-last). cache-time.ts (#451 cache rows): - Remove the unreachable "return iso" after the exhaustive unit loop by dropping the always-matching "second" ladder entry and making the sub-minute case an explicit fallback. Validation: eslint clean on changed files, tsc reports only the two accepted pre-existing test errors, affected vitest suites pass (55 tests), playwright --list parses all 528 specs. * test(e2e): fix repo-create field + force flat view for maven artifact-table tests Two stale-assumption failures that were red across all PRs and on main: - api-comprehensive 'creates test repo' sent `type: local` but the backend requires `repo_type` -> 400 VALIDATION_ERROR (missing field repo_type). - artifact-download and download-url-and-version navigate to a Maven repo and wait for getByRole('table'). Maven repos default to the grouped component view (MavenComponentList), which is not a <table>, so the locator timed out. Navigate with ?view=flat so the flat DataTable renders, matching the tests' row/detail-panel assumptions. Pure test fixes; no app behavior change. * test: raise new-code coverage to >=80% for the v1.2.0 release batch * test(e2e): make v1.2.0 feature specs robust against the e2e environment * test(e2e): fix remaining v1.2.0 interactions spec failures - analytics: combined stat-card locator matched 2 elements, assert .first() - system-config: sidebar is a shadcn data-sidebar container, not a <nav> landmark; key off a guaranteed link and scope scanner-link queries to it - maven-grouped: #444 and #445 seed the same with-zip GAV, so the second seed/retry got 409; tolerate already-deployed (409) in the PUT helper - remote-cached: server-side q filter on a proxy repo can hide the reconstructed cache row; assert the row directly, skip as a backstop since the API-level test is the authoritative #424 guard - repo-dialog-a11y: target the remote repo's actions button by name and retry the menu open (a background list refetch dismissed the dropdown) * test(e2e): bind repo-dialog-a11y to the actual actions trigger The repo row is itself a button whose accessible name concatenates the nested actions-button label, so a substring/.first() match selected the row card (which just selects the repo) instead of the DropdownMenu trigger, so the Edit menuitem never appeared. Match the trigger by exact accessible name. Root-caused from the shard-3 Playwright trace snapshot. --------- Co-authored-by: knowinglyAnonymous <stupidsimpleman8@gmail.com>
|
Shipped in v1.2.0. The changes from this PR were folded into the release integration branch #466, which was squash-merged to main and tagged as v1.2.0. The code is already on main, so closing this as superseded by #466 rather than merging it (re-applying would just duplicate what already shipped). No work lost. See #466 for the consolidated record. |
1 participant
Summary
Two v1.2.0 admin/config features.
System config feature flags (#271). New
SystemConfigProviderfetchesGET /api/v1/system/configonce per session and exposes the result plus derived feature flags viauseSystemConfig/useFeatureFlags. Wired into the obvious gated surfaces:The provider falls back to permissive defaults on fetch error so a transient outage never hides a feature the operator actually configured (scanner-gated surfaces are the deliberate exception and default to hidden).
Rate-limit exemption admin UI (#270). New
/rate-limitsadmin page that shows the effective per-window rate limits and lets admins view, add, and remove exemptions for usernames, service accounts, and CIDR ranges (with client-side CIDR validation). Env-sourced exemptions are shown read-only. The page degrades gracefully when the backend has not yet shipped the exemption-management endpoints (config card and exemptions section each render an "unavailable" state instead of crashing).API clients use the existing
apiFetchwrapper with zod validation at the trust boundary, matching the pattern insrc/lib/api/settings.ts.Test Checklist
UI Changes
E2E specs live under
e2e/suites/interactions/admin/(system-config-feature-flags.spec.ts,rate-limits.spec.ts) and run in theinteractionsproject against the 1.2.0 backend. Unit suite passing locally (vitest run). eslint clean,tsc --noEmitintroduces no new errors.Closes #271
Closes #270