Add Rapid3 manager and split edit button with dropdown options for different versions#46
Conversation
This adds : - A `rapid3.ts` component for managing the embedded Rapid editor (mostly copied from the preexisting `rapid.ts` component) - VITE_RAPID3_URL env variable to point to the location of the new Rapid (currently locally hosted) - Some changes to the startup flow to better insert customizations into the `prepareAsync` -> `initAsync` -> `startAsync` pipeline `edit.vue` currently hardcodes `rapid3Manager` for testing, but we will make it so that the editor can be configurable.
This also updates the .env.example file with the appropriate URLs to fetch from the workspaces-rapid container.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds Rapid v3 support alongside v2: new Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Toolbar
participant Router
participant EditPage
participant Rapid2 as RapidManager
participant Rapid3 as Rapid3Manager
participant Browser
User->>Toolbar: Click Edit on OSW workspace
Toolbar->>Toolbar: Show dropdown (Rapid 2 / Rapid 3)
User->>Toolbar: Select Rapid 3
Toolbar->>Router: Navigate to edit route with ?editor=rapid3
Router->>EditPage: Load page with query
EditPage->>EditPage: Determine target manager (rapid3 vs rapid2)
EditPage->>Rapid2: Check loaded (if present)
alt Rapid2 is loaded and switching to Rapid3
EditPage->>Browser: window.location.reload()
Browser->>EditPage: Full reload with editor=rapid3
EditPage->>Rapid3: load() -> inject assets -> prepare/init
else Direct to Rapid3
EditPage->>Rapid3: load() -> inject assets -> prepare/init
end
Rapid3->>Browser: Render Rapid v3 embedded editor
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
services/rapid3.ts (2)
89-91: Static analysis false positive for innerHTML.The static analysis warning about XSS is a false positive here since the assigned value is a hardcoded string literal with no user input. That said, using
textContentinstead would silence the warning and is semantically clearer for plain text.♻️ Optional: use textContent instead
- container.innerHTML = 'Sorry, your browser is not currently supported.' + container.textContent = 'Sorry, your browser is not currently supported.'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/rapid3.ts` around lines 89 - 91, Replace the innerHTML assignment with a plain text assignment to silence the XSS false positive and be semantically correct: where the code checks Rapid/utilDetect support and sets container.innerHTML = 'Sorry, your browser is not currently supported.', change that to set container.textContent = 'Sorry, your browser is not currently supported.' (keep the container.style.padding = '20px' line unchanged); this targets the block using Rapid.utilDetect() and the container element.
192-198: Consider explicit string conversion forworkspaceIdin headers.
X-Workspaceheader value should be a string. While JavaScript will auto-coerce the numericworkspaceId, explicit conversion improves type safety and clarity.♻️ Proposed fix for explicit type conversion
if (options.headers instanceof Headers) { - options.headers.set('X-Workspace', this.rapidContext.workspaceId) + options.headers.set('X-Workspace', String(this.rapidContext.workspaceId)) options.headers.set('Authorization', tokenHeader) } else if (Array.isArray(options.headers)) { - options.headers.push(['X-Workspace', this.rapidContext.workspaceId]) + options.headers.push(['X-Workspace', String(this.rapidContext.workspaceId)]) options.headers.push(['Authorization', tokenHeader]) } else { - options.headers['X-Workspace'] = this.rapidContext.workspaceId + options.headers['X-Workspace'] = String(this.rapidContext.workspaceId)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/rapid3.ts` around lines 192 - 198, Convert the workspaceId to an explicit string when setting headers in the request-building logic: where options.headers is a Headers instance (the block that calls options.headers.set) and where headers are appended as an array (the Array.isArray branch that pushes ['X-Workspace', ...]), replace the raw this.rapidContext.workspaceId with an explicit string conversion (e.g., String(this.rapidContext.workspaceId) or this.rapidContext.workspaceId.toString()) so the X-Workspace header value is always a string while leaving the Authorization tokenHeader unchanged.pages/workspace/[id]/edit.vue (1)
26-34: Consider early redirect to avoid double page load.The current approach loads the edit page, then immediately triggers
window.location.reload()if switching editors. This causes a visible flash. Consider checking the editor state earlier (e.g., in middleware or before navigation) to avoid the double-load experience.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pages/workspace/`[id]/edit.vue around lines 26 - 34, Move the "other Rapid loaded" check out of the component mount path and into route middleware or a navigation guard (e.g., beforeEach or the component's beforeRouteEnter) so we can redirect before the page renders; specifically check the same variables (datatype, manager, rapidManager, rapid3Manager and otherManager?.loaded.value) and if another manager is loaded perform an early redirect using router.replace (or abort navigation and redirect) instead of calling window.location.reload() from the mounted flow to avoid the double-load/flash.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pages/workspace/`[id]/edit.vue:
- Around line 12-22: route.params.id is a string but manager.init expects a
number; convert and validate it before calling manager.init in onEditorLoaded
(reference workspaceId, onEditorLoaded, manager.init, route.params.id) — parse
the id to a Number (or parseInt) into a new variable (e.g., workspaceIdNum),
check for NaN and handle the error case (log/throw or early return), then pass
the numeric value to manager.init instead of the raw workspaceId string.
In `@services/rapid3.ts`:
- Around line 86-105: The onRapidLoaded method currently calls
context.prepareAsync() without handling rejections; update the `#onRapidLoaded`
function to chain a .catch(...) (or use try/catch if converted to async) on
context.prepareAsync() to handle failures: log the error (e.g., console.error or
a logger), set a visible error state or update container.innerHTML with a
user-facing message, and ensure this.loaded.value is set appropriately (false or
cleared) so the UI doesn't stay in perpetual loading; keep the existing
.then(...) branch that sets this.loaded.value = true on success.
---
Nitpick comments:
In `@pages/workspace/`[id]/edit.vue:
- Around line 26-34: Move the "other Rapid loaded" check out of the component
mount path and into route middleware or a navigation guard (e.g., beforeEach or
the component's beforeRouteEnter) so we can redirect before the page renders;
specifically check the same variables (datatype, manager, rapidManager,
rapid3Manager and otherManager?.loaded.value) and if another manager is loaded
perform an early redirect using router.replace (or abort navigation and
redirect) instead of calling window.location.reload() from the mounted flow to
avoid the double-load/flash.
In `@services/rapid3.ts`:
- Around line 89-91: Replace the innerHTML assignment with a plain text
assignment to silence the XSS false positive and be semantically correct: where
the code checks Rapid/utilDetect support and sets container.innerHTML = 'Sorry,
your browser is not currently supported.', change that to set
container.textContent = 'Sorry, your browser is not currently supported.' (keep
the container.style.padding = '20px' line unchanged); this targets the block
using Rapid.utilDetect() and the container element.
- Around line 192-198: Convert the workspaceId to an explicit string when
setting headers in the request-building logic: where options.headers is a
Headers instance (the block that calls options.headers.set) and where headers
are appended as an array (the Array.isArray branch that pushes ['X-Workspace',
...]), replace the raw this.rapidContext.workspaceId with an explicit string
conversion (e.g., String(this.rapidContext.workspaceId) or
this.rapidContext.workspaceId.toString()) so the X-Workspace header value is
always a string while leaving the Authorization tokenHeader unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1ce0a811-a358-43ea-b27d-537bb2a497ff
📒 Files selected for processing (5)
.env.examplecomponents/dashboard/Toolbar.vuepages/workspace/[id]/edit.vueservices/index.tsservices/rapid3.ts
| const workspaceId = route.params.id | ||
| const datatype = route.query.datatype | ||
| const editor = route.query.editor | ||
| const editorContainer = ref(null) | ||
| const manager = datatype === 'osw' ? rapidManager : pathwaysManager | ||
|
|
||
| const oswManager = (editor === 'rapid3' && rapid3Manager) ? rapid3Manager : rapidManager | ||
| const manager = datatype === 'osw' ? oswManager : pathwaysManager | ||
|
|
||
| function onEditorLoaded() { | ||
| editorContainer.value.appendChild(manager.containerNode) | ||
| manager.init(workspaceId) |
There was a problem hiding this comment.
Type mismatch: workspaceId is a string but init() expects a number.
route.params.id returns a string, but Rapid3Manager.init(workspaceId: number) expects a numeric argument. This could cause issues with strict type checks or if the workspace ID is used in numeric comparisons.
🛡️ Proposed fix to parse workspaceId
const route = useRoute()
-const workspaceId = route.params.id
+const workspaceId = Number(route.params.id)
const datatype = route.query.datatype🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pages/workspace/`[id]/edit.vue around lines 12 - 22, route.params.id is a
string but manager.init expects a number; convert and validate it before calling
manager.init in onEditorLoaded (reference workspaceId, onEditorLoaded,
manager.init, route.params.id) — parse the id to a Number (or parseInt) into a
new variable (e.g., workspaceIdNum), check for NaN and handle the error case
(log/throw or early return), then pass the numeric value to manager.init instead
of the raw workspaceId string.
| #onRapidLoaded() { | ||
| const container = this.containerNode; | ||
|
|
||
| if (typeof Rapid === 'undefined' || !Rapid.utilDetect().support) { | ||
| container.innerHTML = 'Sorry, your browser is not currently supported.' | ||
| container.style.padding = '20px' | ||
|
|
||
| } else { | ||
| const context = new Rapid.Context() | ||
| context.embed(true); // hide the account management control | ||
| context.containerNode = container | ||
| context.assetPath = this.#baseUrl | ||
|
|
||
| this.rapidContext = context | ||
| context.prepareAsync() | ||
| .then(() => { | ||
| this.loaded.value = true | ||
| }) | ||
| } | ||
| } |
There was a problem hiding this comment.
Missing error handling for prepareAsync() rejection.
If prepareAsync() rejects, the promise error goes unhandled and loaded.value never becomes true, leaving the UI in a perpetual loading state with no user feedback.
🛡️ Proposed fix to handle preparation failure
} else {
const context = new Rapid.Context()
context.embed(true); // hide the account management control
context.containerNode = container
context.assetPath = this.#baseUrl
this.rapidContext = context
context.prepareAsync()
.then(() => {
this.loaded.value = true
})
+ .catch((err) => {
+ console.error('Failed to prepare Rapid3 context:', err)
+ container.textContent = 'Failed to initialize the editor. Please refresh the page.'
+ container.style.padding = '20px'
+ })
}🧰 Tools
🪛 ast-grep (0.41.1)
[warning] 89-89: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: container.innerHTML = 'Sorry, your browser is not currently supported.'
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 89-89: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: container.innerHTML = 'Sorry, your browser is not currently supported.'
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/rapid3.ts` around lines 86 - 105, The onRapidLoaded method currently
calls context.prepareAsync() without handling rejections; update the
`#onRapidLoaded` function to chain a .catch(...) (or use try/catch if converted to
async) on context.prepareAsync() to handle failures: log the error (e.g.,
console.error or a logger), set a visible error state or update
container.innerHTML with a user-facing message, and ensure this.loaded.value is
set appropriately (false or cleared) so the UI doesn't stay in perpetual
loading; keep the existing .then(...) branch that sets this.loaded.value = true
on success.
Elsewhere in the code (`index.ts` and `edit.vue`) we have guards to prevent trying to instantiate Rapid3 if there is no Rapid3 URL set. This commit just checks for that in the new dropdown code as well, so that we won't offer a Rapid3 option if there is not Rapid3Manager.
3 participants
This commit adds:
rapid3.tsRapid3Manager for instantiating the new version of rapid.env.examplewith the example urls for loading different versions from workspaces-rapidindex.tsis showing up as having a lot of changes in it - I believe this was just from being converted from CRLF to LF newlines. The only significant change was the addition of the Rapid3 manager at the end.The dropdown allows us to offer multiple editors from here.
If the user clicks the main part of the button, they just get the default Rapid 2.
Overview
This PR adds support for Rapid v3 alongside Rapid v2 by introducing a new Rapid3Manager and converting the Edit button into a split dropdown so users can choose "Rapid 2 (default)" or "Rapid 3 (beta)". Rapid 2 remains the default; Rapid 3 is offered conditionally only when a Rapid 3 URL is configured.
Changes by File
.env.example
VITE_RAPID3_URL(example:https://rapid.workspaces-dev.sidewalks.washington.edu/rapid3/)VITE_RAPID_URLto point to the Rapid v2 path (.../rapid2/)components/dashboard/Toolbar.vue
rapid3Manageris presenteditRoute(Rapid 2)editor: 'rapid3'query)isOsw,rapid3Available(guard), andeditRouteRapid3rapid3Managerto conditionally show Rapid 3 optionpages/workspace/[id]/edit.vue
route.query.editorrapid3Managerand computesoswManagerto choose betweenrapidManager(v2) andrapid3Manager(v3)services/index.ts
rapid3Urlfromimport.meta.env.VITE_RAPID3_URLrapid3ManagerasRapid3Manager | null— initialized only whenrapid3Urlis truthyservices/rapid3.ts (new)
Rapid3Managerclass to manage embedded Rapid v3:loadedandcontainerNodeload()injects Rapid v3 CSS/JS (idempotent with error handling)init(workspaceId)runs Rapid’s prepare/init/start sequence and applies patchesswitchWorkspace(workspaceId)updates context, dispatches synthetichashchange, and callsresetAsync()X-WorkspaceandAuthorization: Bearer <token>headers, override authenticated checks to use TDEI auth, and stub userDetailsOther notes
rapid3Managerexists (i.e., whenVITE_RAPID3_URLis configured).