Skip to content

Security issues and bugs from Pixelbot implementation #3830

@RichDom2185

Description

@RichDom2185

Follow up from #3754

{
  "comment_content": "There are now two identical `AnchorButton` chatbot icons rendered: one inside `.bot-area` and another after the `{isPop && <ChatBox .../>}` block. This will duplicate the draggable handle/button in the UI and can cause overlapping click/drag behavior. Remove the extra `AnchorButton` (or refactor so the icon is rendered only once).",
  "comment_type": "bug",
  "file_location": "src/pages/sicp/subcomponents/chatbot/Chatbot.tsx",
  "start_line": 156,
  "end_line": 168,
  "fixed": false,
  "severity": "critical"
}
{
  "comment_content": "`Save Feedback URL` calls `onSave(props.courseConfiguration)`, but `courseConfiguration` may still contain the previous `feedbackUrl` value if the user clicks Save immediately after typing (state updates are async). Persist the same `updatedConfig` you build in `onChange` (e.g. keep a local draft like the prompts, or compute-and-save in the click handler) to avoid saving stale data.",
  "comment_type": "bug",
  "file_location": "src/pages/academy/adminPanel/subcomponents/PixelbotConfigPanel.tsx",
  "start_line": 162,
  "end_line": 179,
  "fixed": false,
  "severity": "moderate"
}
{
  "comment_content": "`resetChat()` is only invoked once on mount (effect has `[]` deps). Because `useTokens({ throwWhenEmpty: false })` can return empty tokens on first render, `resetChat()` may return early and never re-run when tokens become available, so chat history/maxContentSize won’t initialize. Re-run `resetChat()` when tokens become available (e.g. depend on `tokens.accessToken`/`tokens.refreshToken`).",
  "comment_type": "bug",
  "file_location": "src/pages/academy/ragChatbot/RagChatBox.tsx",
  "start_line": 161,
  "end_line": 165,
  "fixed": false,
  "severity": "moderate"
}
{
  "comment_content": "`clampPosition` depends on `window.innerWidth/innerHeight`, but the chatbot never re-clamps its position on viewport resize. After resizing, the draggable widget can end up partially off-screen and become hard to reach. Add a resize listener (similar to the SICP chatbot) that updates `position` via `clampPosition` when `isPop`/`isExpanded` or the viewport changes.",
  "comment_type": "bug",
  "file_location": "src/pages/academy/ragChatbot/RagChatbot.tsx",
  "start_line": 23,
  "end_line": 38,
  "fixed": false,
  "severity": "moderate"
}
{
  "comment_content": "`feedbackUrl` is inserted directly into an `<a href={feedbackUrl}>` from session/course config. Without validating allowed protocols, a misconfigured/malicious value like `javascript:...` could execute script when clicked. Restrict to safe protocols (e.g. http/https) or normalize/validate the URL before rendering.",
  "comment_type": "security",
  "file_location": "src/pages/academy/ragChatbot/RagChatBox.tsx",
  "start_line": 183,
  "end_line": 191,
  "fixed": false,
  "severity": "critical"
}

Originally posted by @RichDom2185 in #3754 (review)

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions