FEAT: URL-driven views and shareable deep links in the GUI#1994
Open
adrian-gavrila wants to merge 7 commits into
Open
FEAT: URL-driven views and shareable deep links in the GUI#1994adrian-gavrila wants to merge 7 commits into
adrian-gavrila wants to merge 7 commits into
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…fresh Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
5c9f2fe to
8db537a
Compare
Resolve dependency conflicts: keep main's React 19 bump and re-add react-router-dom. Silence the new react-hooks/set-state-in-effect rule on the intentional loadedAttack cleanup. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR makes the PyRIT GUI URL-driven so view state (including selected attacks/conversations and history filters) survives refreshes and can be shared via deep links, while also ensuring production refreshes on deep routes don’t 404 behind the FastAPI static mount.
Changes:
- Introduces
react-router-domrouting and refactorsApp.tsxto derive view/attack state from the URL (including deep links to attacks/conversations). - Persists History filters in the query string with encode/decode helpers and adds unit/e2e coverage.
- Adds
SPAStaticFilesto serveindex.htmlfor unmatched non-API paths, preserving SPA refresh behavior, plus backend tests for/apivs non-API boundaries.
Reviewed changes
Copilot reviewed 15 out of 16 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
tests/unit/backend/test_main.py |
Adds unit coverage for SPA static fallback behavior and /api namespace boundary. |
pyrit/backend/main.py |
Adds SPAStaticFiles and mounts it for production frontend serving to support deep-link refresh. |
frontend/src/setupTests.ts |
Polyfills TextEncoder/TextDecoder for jsdom compatibility with router import-time usage. |
frontend/src/main.tsx |
Wraps the app with BrowserRouter (inside AuthProvider). |
frontend/src/components/History/historyFilters.ts |
Adds query-string encode/decode helpers for shareable History filters. |
frontend/src/components/History/historyFilters.test.ts |
Unit tests for History filter URL round-tripping behavior. |
frontend/src/components/Chat/AttackNotFound.tsx |
Adds a not-found UI for invalid/missing attack deep links. |
frontend/src/components/Chat/AttackNotFound.test.tsx |
Unit tests for the Attack-not-found UI behavior. |
frontend/src/components/Chat/AttackNotFound.styles.ts |
Styles for the Attack-not-found view. |
frontend/src/auth/AuthProvider.tsx |
Captures requested URL into MSAL state and restores it post-login with an internal-path guard. |
frontend/src/auth/AuthProvider.test.tsx |
Adds tests for state capture, deep-link restore, and open-redirect guard behavior. |
frontend/src/App.tsx |
Major refactor: URL ↔ state orchestrator for views, deep-linked attacks, and history query filters. |
frontend/src/App.test.tsx |
Updates tests for routed structure and adds coverage for deep-link hydration and filter URL behavior. |
frontend/package.json |
Adds react-router-dom dependency. |
frontend/package-lock.json |
Locks react-router(-dom) transitive deps and engines. |
frontend/e2e/routing.spec.ts |
New Playwright e2e coverage asserting address-bar behavior: deep links, reload, back/forward, filters. |
Files not reviewed (1)
- frontend/package-lock.json: Language not supported
Comment on lines
+57
to
+62
| * open redirect. Backslashes are treated as slashes because the URL parser | ||
| * normalizes "\" to "/". | ||
| */ | ||
| function isSafeInternalPath(path: string): boolean { | ||
| return path.startsWith('/') && !path.startsWith('//') && !path.startsWith('/\\') | ||
| } |
Comment on lines
+193
to
+209
| attacksApi | ||
| .getAttack(routeAttackId) | ||
| .then(attack => { | ||
| if (cancelled) return | ||
| setLoadedAttack({ | ||
| id: routeAttackId, | ||
| mainConversationId: attack.conversation_id, | ||
| labels: attack.labels ?? {}, | ||
| target: attack.target ?? null, | ||
| relatedConversationIds: attack.related_conversation_ids ?? [], | ||
| status: 'success', | ||
| }) | ||
| }) | ||
| .catch(() => { | ||
| if (cancelled) return | ||
| setLoadedAttack({ | ||
| id: routeAttackId, |
Comment on lines
+186
to
+190
| def test_unknown_api_path_still_404(self, spa_client: TestClient) -> None: | ||
| """Test that an unknown /api path stays a real 404 instead of being masked by index.html.""" | ||
| resp = spa_client.get("/api/bogus") | ||
| assert resp.status_code == 404 | ||
| assert "spa-index" not in resp.text |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Today all view state in the frontend GUI lives in React
useStateinApp.tsx, so refreshing the page resets you back to Home and there is no way to share a link to a specific attack or conversation. This PR makes the GUI URL-driven: the current view and the data being viewed are encoded in the browser URL, so refreshes restore exactly what you were looking at and links are copy-pasteable.The change consists of these main components:
react-router-dom@7with a path-basedBrowserRouter(notHashRouter, since MSAL uses the URL hash). Top-level views map to/,/chat,/history,/config./attacks/<id>and/attacks/<id>/conversations/<conv>hydrate the chat window directly from the URL.App.tsxbecomes the URL<->state orchestrator while preserving the existing behaviors: the cross-target guard, operator-label locking,isLoadingAttackhandling, related-conversation selection, and the "re-opening the same attack" fetch optimization. Invalid/missing ids render a not-found view.SPAStaticFilessubclass servesindex.htmlfor unmatched non-API paths so a hard refresh on a deep link does not 404 behind the FastAPI static mount.navigateToLoginRequestUrl, so deep-link restore across the login round-trip is done manually: the requested path is captured as MSALstateand restored afterhandleRedirectPromise. BecauseBrowserRoutermounts insideAuthProvider, the URL is corrected before the router reads it, so no full reload is needed. AnisSafeInternalPathguard restricts restore to same-origin root-relative paths to avoid an open redirect.Notes for reviewers
AuthProvidergates on the initialized MSAL instance.SPAStaticFilesapi-path guard normalizesos.septo/before matching: on Windows the path arrives with backslashes, so a naivestartswith("api/")check silently leaks the SPA for real/api/...404s. The accompanying test pins this boundary.Tests and Documentation
AttackNotFound.test.tsx,historyFilters.test.ts, and AuthProvider tests for state capture, deep-link restore, and the open-redirect guard (including a backslash-prefixed state). UpdatedApp.test.tsxfor the routed structure. Full frontend suite: 636+ passing.frontend/e2e/routing.spec.ts(first e2e coverage that asserts on the browser URL): history-filter round-trip + reload, deep-link hydration, not-found, and back/forward navigation. 4/4 passing.tests/unit/backend/test_main.py::TestSPAStaticFilescovering SPA fallback, the/api404 boundary, and the/apikeys-style prefix case.npm run lint,npm run type-check,npm test, and the affected backend pytest all pass. No doc/JupyText samples are affected by this change.