Fix XSS, inline styles, dialog a11y, and minor cleanup #3

Open
opened 2026-03-21 16:45:16 +00:00 by forgejo_admin · 0 comments

Type

Bug

Repo

forgejo_admin/minio-playground

What Broke

QA review of PR #2 found 8 issues:

  1. XSS — URL params inserted into innerHTML without HTML escaping
  2. Inline onclick handler on download button (inconsistent with addEventListener pattern)
  3. Inline style attributes in detail.html and browse.html (should be CSS classes)
  4. No focus trap on delete confirmation dialog
  5. Redundant ternary in renderObjectBrowser() — both branches identical
  6. No Escape key handler for dialog dismissal
  7. Upload nav link hardcoded to assets bucket regardless of current context
  8. Missing ## Related section in PR body

Repro Steps

  1. Open browse.html?bucket=<script>alert(1)</script> — XSS via innerHTML
  2. Open detail.html — inspect download button, find inline onclick
  3. Open detail.html or browse.html — inspect elements, find inline style attributes
  4. Open delete dialog — press Tab repeatedly, focus escapes the dialog
  5. Open delete dialog — press Escape, dialog does not close

Expected Behavior

  1. All URL params HTML-escaped before innerHTML insertion
  2. All event handlers use addEventListener
  3. All styling in CSS classes, no inline style attributes
  4. Delete dialog traps focus within itself
  5. Escape key closes the dialog
  6. Upload nav reflects current bucket context
  7. No dead code (redundant ternary removed)

Environment

  • minio-playground main branch (post PR #2 merge)
  • Any browser at 390px viewport

Lineage

plan-pal-e-platform → Phase 26 (QA nits from PR #2)

File Targets

  • app.js — XSS fix, onclick removal, redundant ternary, upload nav
  • detail.html, browse.html — move inline styles to CSS classes
  • style.css — add CSS classes for moved styles
  • Dialog component — focus trap + Escape key handler

Acceptance Criteria

  • No innerHTML with unescaped user input
  • No inline onclick handlers
  • No inline style attributes
  • Delete dialog has focus trap + Escape key dismiss
  • Upload nav reflects current bucket
  • All pages render at 390px without horizontal scroll

Checklist

  • PR opened
  • No unrelated changes
  • phase-pal-e-platform-26-minio-playground — source phase
### Type Bug ### Repo `forgejo_admin/minio-playground` ### What Broke QA review of PR #2 found 8 issues: 1. XSS — URL params inserted into `innerHTML` without HTML escaping 2. Inline `onclick` handler on download button (inconsistent with `addEventListener` pattern) 3. Inline `style` attributes in `detail.html` and `browse.html` (should be CSS classes) 4. No focus trap on delete confirmation dialog 5. Redundant ternary in `renderObjectBrowser()` — both branches identical 6. No Escape key handler for dialog dismissal 7. Upload nav link hardcoded to `assets` bucket regardless of current context 8. Missing `## Related` section in PR body ### Repro Steps 1. Open `browse.html?bucket=<script>alert(1)</script>` — XSS via innerHTML 2. Open `detail.html` — inspect download button, find inline `onclick` 3. Open `detail.html` or `browse.html` — inspect elements, find inline `style` attributes 4. Open delete dialog — press Tab repeatedly, focus escapes the dialog 5. Open delete dialog — press Escape, dialog does not close ### Expected Behavior 1. All URL params HTML-escaped before innerHTML insertion 2. All event handlers use `addEventListener` 3. All styling in CSS classes, no inline `style` attributes 4. Delete dialog traps focus within itself 5. Escape key closes the dialog 6. Upload nav reflects current bucket context 7. No dead code (redundant ternary removed) ### Environment - minio-playground main branch (post PR #2 merge) - Any browser at 390px viewport ### Lineage `plan-pal-e-platform` → Phase 26 (QA nits from PR #2) ### File Targets - `app.js` — XSS fix, onclick removal, redundant ternary, upload nav - `detail.html`, `browse.html` — move inline styles to CSS classes - `style.css` — add CSS classes for moved styles - Dialog component — focus trap + Escape key handler ### Acceptance Criteria - [ ] No innerHTML with unescaped user input - [ ] No inline onclick handlers - [ ] No inline style attributes - [ ] Delete dialog has focus trap + Escape key dismiss - [ ] Upload nav reflects current bucket - [ ] All pages render at 390px without horizontal scroll ### Checklist - [ ] PR opened - [ ] No unrelated changes ### Related - `phase-pal-e-platform-26-minio-playground` — source phase
Sign in to join this conversation.
No labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
forgejo_admin/minio-playground#3
No description provided.