fix: prevent duplicate submissions and harden join idempotency#35
fix: prevent duplicate submissions and harden join idempotency#35Flashl3opard wants to merge 2 commits intoalphaonelabs:mainfrom
Conversation
WalkthroughAdds client-side action locks to prevent concurrent submissions across three frontend pages (course, login, teach), introduces a "My Hosted Activities" section with versioned dashboard loading on teach.html, and hardens the backend join handler to detect and short-circuit duplicate enrollments before/after insertion. Also includes HTML/CSS reformatting and minor markup adjustments. Changes
Sequence Diagram(s)mermaid User->>Browser: Click "Join" (actId) Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
There was a problem hiding this comment.
Pull request overview
This PR reduces duplicate user actions in the UI and makes /api/join safe to call repeatedly, preventing duplicate enrollments and inconsistent client states.
Changes:
- Added client-side in-flight action locks for submit/click handlers (login/register, host actions, join).
- Added stale-response protection for the hosted-activities refresh flow in Host Hub.
- Updated
/api/jointo return an idempotent “Already joined activity” outcome when an enrollment already exists.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/worker.py |
Adds idempotent join behavior by checking for an existing enrollment and handling unique-constraint conflicts. |
public/teach.html |
Adds in-flight guarding for create-activity/session actions and prevents stale hosted-activities refresh from overwriting newer state. |
public/login.html |
Adds in-flight guarding to prevent duplicate login/register submissions. |
public/course.html |
Adds in-flight guarding for “Join Activity” to prevent duplicate join requests from rapid clicks. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@public/course.html`:
- Around line 89-91: The Join CTA button should have an explicit type to prevent
accidental form submission; update the element with id "btn-join" (the button
that calls joinActivity()) to include type="button" so it will not act as a
submit button if moved into a form.
In `@public/login.html`:
- Around line 35-38: Add explicit button types and proper label-input
associations: change the tab buttons (ids "tab-login" and "tab-register") to
include type="button" to prevent form submission, and update each form field so
every <label> has a matching for attribute that points to the corresponding
input's id (ensure inputs like email, password, register-name, etc. have unique
id values and labels reference them). Apply the same type="button" and label-for
fixes to the other tab/button elements and all login/register field label-input
pairs mentioned (lines 45-48, 51-54, 71-74, 77-80, 83-86, 89-92).
- Line 90: The client-side password input (id="r-password") currently enforces
minlength="6" while the backend requires passwords of at least 8 characters;
update the HTML input to require minlength="8" (and adjust any related
client-side validation messages) so the registration form matches the backend
password length check (backend password length >= 8).
In `@public/teach.html`:
- Around line 72-75: The labels are not programmatically associated with their
inputs; update each label to include a for attribute that matches the
corresponding input's id (e.g., label for="a-title" to pair with input
id="a-title") and ensure every input element (e.g., a-title, a-description,
a-language, etc.) has a unique id that the matching label references; apply this
same for/id pairing pattern to all other label/input pairs mentioned (lines
around 78–81, 85–88, 96–99, 105–108, 116–119, 139–143, 146–149, 153–156,
158–161, 164–167, 170–173) so screen readers and click-to-focus work correctly.
- Around line 249-256: The loadHostedActivities function assumes
fetch('/api/dashboard') always succeeds; wrap the fetch/response parsing in a
try/catch, check res.ok before calling res.json(), and handle errors by setting
hostedActivities = [] (or preserving prior state), updating
document.getElementById('hosted-count') with a clear error or zero state, and
optionally surface a user-facing error message (e.g., show an auth
expired/network error banner). Apply the same pattern to the other similar fetch
block at the later section (lines ~263-267) so both network failures and non-OK
responses are handled explicitly using hostedLoadVersion, hostedActivities, and
the 'hosted-count' DOM update.
- Around line 278-279: The code concatenates (fmtLabel[a.format] || a.format)
directly into an HTML string, creating an XSS sink; update the generation to
sanitize that fallback by escaping special characters before inserting into
innerHTML (or, alternatively, build the elements and set span.textContent
instead of using innerHTML). Specifically, ensure the value used for the first
span (where fmtLabel[a.format] || a.format is referenced) is passed through a
safe escapeHtml utility (or created via document.createElement + textContent) so
fmtLabel and the a.format fallback cannot inject HTML.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: alphaonelabs/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 694a6791-4ddb-49aa-af73-cec1fc68096f
📒 Files selected for processing (4)
public/course.htmlpublic/login.htmlpublic/teach.htmlsrc/worker.py
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
public/login.html (1)
90-92:⚠️ Potential issue | 🟡 MinorClient-side password validation still misaligned with backend.
The input specifies
minlength="6"and the placeholder says "min 6 characters," butsrc/worker.py(line 656) rejects passwords shorter than 8 characters. This mismatch causes users to submit passwords that pass client validation but fail server-side, leading to a poor user experience.✨ Suggested fix
- <input id="r-password" type="password" autocomplete="new-password" required minlength="6" + <input id="r-password" type="password" autocomplete="new-password" required minlength="8" class="w-full px-4 py-2.5 rounded-xl border border-slate-200 focus:outline-none focus:ring-2 focus:ring-brand/40 text-sm" - placeholder="min 6 characters" /> + placeholder="min 8 characters" />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@public/login.html` around lines 90 - 92, The client-side password field (input id "r-password" in public/login.html) currently uses minlength="6" and "min 6 characters" placeholder but the server-side check in src/worker.py enforces an 8-character minimum; update the frontend to match the backend by changing the input's minlength to "8" and its placeholder text to "min 8 characters", and also update any client-side JS validation that references a 6-char minimum so all checks align with the server-side rule enforced in src/worker.py.public/teach.html (1)
289-290:⚠️ Potential issue | 🟠 MajorEscape the format fallback to prevent potential XSS.
Line 290 inserts
(fmtLabel[a.format] || a.format)directly into the HTML string. If unexpected or malicious data reachesa.formatfrom the backend, this becomes an XSS vector. ThefmtLabellookup is safe, but the fallback toa.formatshould be escaped.🛡️ Suggested fix
'<span class="badge ' + tc + '">' + esc(a.type) + '</span>' + - '<span>' + (fmtLabel[a.format] || a.format) + '</span>' + + '<span>' + esc(fmtLabel[a.format] || a.format) + '</span>' + '<span>👥 ' + a.participant_count + '</span>' +As per coding guidelines,
**/*.html: "Review HTML templates for ... XSS risks from unescaped user content..."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@public/teach.html` around lines 289 - 290, The fallback output (fmtLabel[a.format] || a.format) is not escaped and can lead to XSS if a.format is attacker-controlled; update the template to pass the fallback through the existing esc(...) sanitizer so it becomes (fmtLabel[a.format] || esc(a.format)) or equivalently esc(fmtLabel[a.format] || a.format), ensuring you use the same esc function used for a.type; modify the expression near where fmtLabel and a.format are concatenated into the HTML string so the a.format fallback is always escaped.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@public/course.html`:
- Around line 138-141: The dynamically generated logout button in nav.innerHTML
lacks an explicit type attribute; update the string assigned to nav.innerHTML
(the expression building the greeting, Dashboard link, and Logout button) to
include type="button" on the Logout button so it matches static buttons and
avoids accidental form submissions (reference: nav.innerHTML and logout()).
In `@public/teach.html`:
- Line 45: The Logout button element that calls logout() currently lacks an
explicit type which can cause accidental form submissions later; update the
<button onclick="logout()" class="text-slate-400 text-sm hover:text-red-500">
element to include type="button" so it explicitly behaves as a non-submit button
(i.e., add type="button" to the button that invokes the logout() handler).
---
Duplicate comments:
In `@public/login.html`:
- Around line 90-92: The client-side password field (input id "r-password" in
public/login.html) currently uses minlength="6" and "min 6 characters"
placeholder but the server-side check in src/worker.py enforces an 8-character
minimum; update the frontend to match the backend by changing the input's
minlength to "8" and its placeholder text to "min 8 characters", and also update
any client-side JS validation that references a 6-char minimum so all checks
align with the server-side rule enforced in src/worker.py.
In `@public/teach.html`:
- Around line 289-290: The fallback output (fmtLabel[a.format] || a.format) is
not escaped and can lead to XSS if a.format is attacker-controlled; update the
template to pass the fallback through the existing esc(...) sanitizer so it
becomes (fmtLabel[a.format] || esc(a.format)) or equivalently
esc(fmtLabel[a.format] || a.format), ensuring you use the same esc function used
for a.type; modify the expression near where fmtLabel and a.format are
concatenated into the HTML string so the a.format fallback is always escaped.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: alphaonelabs/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 87bfa8a9-cad0-470b-be9d-8a56117d4a86
📒 Files selected for processing (4)
public/course.htmlpublic/login.htmlpublic/teach.htmlsrc/worker.py
Summary
Prevent duplicate user actions and make join operations idempotent.
Changes
[/api/join]with idempotent handling on backend.Problem
Rapid clicks / repeated submits could trigger duplicate requests and inconsistent UI states.
Testing
login/registercannot be submitted repeatedly while request is in progress.activity/sessioncreation actions are locked during in-flight requests.Purpose
Prevent duplicate user submissions and enforce idempotent join behavior by adding frontend in-flight request guards and backend idempotent handling for join operations.
Key Modifications
Frontend
Backend
Functional Impact