The attachment grid's <img> thumbnail pointed at the same
download_attachment.php URL as the full-size original, so previewing a
multi-MB photo attachment cost a full multi-MB download just to render a
small grid preview. loading="lazy" only deferred off-screen images; it
never reduced per-image transfer size.
Generate a resized JPEG thumbnail (longest side capped at 300px) via GD
at upload time, from the same metadata-stripped image stripImageMetadata()
already produces, reusing its decompression-bomb guard (~40MP decode cap).
Store the thumbnail's filename in a new nullable ticket_attachments.
thumbnail_filename column (migration 005); NULL means no thumbnail exists
(non-image, GD unavailable, or an attachment predating this change) and
callers fall back to the full-size original.
download_attachment.php serves the thumbnail when requested via
?thumb=1 and one exists, falling back to the original otherwise. The
attachments grid now requests thumb=1 for its <img> preview; the
lightbox link is unchanged and still opens the full-size original.
delete_attachment.php removes the thumbnail file alongside the original,
and cleanup_orphan_uploads.php's orphan lookup now also matches
thumbnail_filename so generated thumbnails aren't swept up as orphans.
Verified against real MariaDB + GD: a 1600x1200 test JPEG produced a
300x225 thumbnail at ~1.8KB vs. the 52KB original (~29x smaller);
confirmed the serving logic picks the thumbnail for image attachments
with one, falls back to the original for a non-image attachment even
when thumb=1 is requested, and that the updated orphan-cleanup lookup
matches both the original and thumbnail filename (and correctly finds
neither for an unrelated filename).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0117oBw2jN4kALYeS8HPq4zV
The 4 bulk-action confirm buttons (close/assign/priority/status) called
their performBulk*() handler directly on click with no in-flight guard.
Double-clicking a confirm button fired two concurrent POST /api/
bulk_operation.php requests for the same ticket IDs, duplicating the
close-reason comment, audit-log entry, and Matrix notification on every
affected ticket.
Each performBulk*() function now takes the clicked button, no-ops if
it's already disabled, disables it before firing the request, and
re-enables it in .finally() regardless of outcome.
Verified by extracting performBulkAssign() from the real source and
driving it with a mock lt.api.post() that never resolves until told to:
a simulated rapid double-click fired exactly one request (the second
call was a no-op while the button was disabled), and a subsequent click
after the request resolved correctly fired a new request.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0117oBw2jN4kALYeS8HPq4zV
A comment accompanying a status change (required or user-supplied) was
posted via a separate, independent HTTP call/write (add_comment.php,
or a second add_comment call in lt.ticketStatus.submit()'s
requires_comment retry path) before the status update itself. A failure
partway through — or the client never issuing the second call — could
leave a "reason" comment persisted with no matching status change, or
vice versa, with no rollback tying the two together.
api/update_ticket.php and api/ticket_status_api.php now post the comment
and apply the status update inside one transaction, rolling back both on
any failure. assets/js/ticket.js and lt.ticketStatus.submit() in
assets/js/base.js no longer make a separate add_comment.php call; they
pass the comment directly to update_ticket.php, which persists it
server-side alongside the status change.
Verified against real MariaDB by extracting the live ApiTicketController
and the ticket_status_api.php transaction logic and running them
directly: a forced optimistic-lock conflict correctly rolled back both
the comment and the status change, and a successful call persisted
exactly one comment alongside the status change.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0117oBw2jN4kALYeS8HPq4zV
The markdown toolbar offered bold/italic/code/heading/list/quote/link
but no table option, despite README describing table rendering as a
supported feature — the parser already renders manually-typed table
syntax correctly, this was purely a discoverability gap for a user who
wouldn't otherwise know the exact `| Header | Header |` / `|---|---|`
syntax to type from scratch.
Added toolbarTable(), which inserts a 2-column starter template (with
a leading newline only when needed, matching the table syntax's
requirement of a full line to itself) matching exactly what
parseMarkdownTables()'s detection regex expects, wired into the
toolbar's existing data-toolbar-action dispatch. Verified via jsdom
that the inserted template parses into a real HTML table.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Lhz7pGMaoTfL5sdYS5XiKv
lt.markdown was confirmed dead code app-wide (zero callers outside
base.js itself — markdown.js's parseMarkdown() is what's actually
wired up everywhere). The issue flagged its link handler as lacking a
URL-protocol allowlist unlike markdown.js's equivalent; checking the
current code, that link handler already restricts to http(s)/relative/
hash URLs (blocking javascript:/data: URIs) — the allowlist claim
didn't match what's actually there. Since the module is unused either
way, and its own doc comment invites exactly the kind of future
misuse the issue warned about ("For full GFM, swap in marked.js"),
deleted it outright rather than hardening dead code, removing the
landmine permanently instead of leaving an unused copy that could
still drift out of sync with markdown.js in some other way later.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Lhz7pGMaoTfL5sdYS5XiKv
ESLint flagged ~20 empty catch blocks and 4 loose-equality comparisons
in assets/js/. Auditing each: all ~19 remaining empty catches are
localStorage/sessionStorage access (persisted tab/theme/column-
visibility state, recent command-palette entries) or the terminal
beep's AudioContext calls — genuinely intentional best-effort UX
affordances that must silently no-op if storage is disabled/full or
audio is blocked, not oversights. One (a viewport-change listener
callback) was a real gap: swallowing an arbitrary caller-supplied
callback's exception could hide a genuine bug, so that one now logs
via console.error instead.
Documented the storage/audio convention once in a file-level comment
rather than repeating the same explanation on ~19 near-identical
one-line try/catches. All 4 flagged loose-equality comparisons turned
out to be `== null`/`!= null` checks — the one loose-equality idiom
that's deliberately safe (catches both null and undefined in one
comparison; ESLint's own eqeqeq rule has a "smart" mode specifically
for this). Converting them to strict equality would have been a
behavior change (no longer catching undefined), not a fix, so switched
.eslintrc.json's eqeqeq rule to "smart" instead — flags every other
loose comparison as before, correctly stops flagging this one safe
idiom.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Lhz7pGMaoTfL5sdYS5XiKv
updatePreview() was bound directly to the comment textarea's 'input'
event with no debounce, re-running the full markdown parser (regex
passes for headings, tables, links, footnotes, etc.) on every single
keystroke.
Wrapped it with the existing lt.debounce() helper (150ms) — the
initial preview render on enabling the toggle still happens
immediately; only the per-keystroke live updates are debounced.
Verified via jsdom with real timers: 10 rapid keystrokes within the
debounce window produce exactly one parse call instead of ten.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Lhz7pGMaoTfL5sdYS5XiKv
Same root pattern as the earlier chart click-to-filter bug (#29):
performAdvancedSearch() built a brand-new URLSearchParams from only the
form's own fields and navigated to it, silently dropping any active
filter the form doesn't represent (e.g. a category/type filter applied
via a dashboard quick-filter pill or stats-widget click).
populateCurrentFilters() also only read search/status back out of the
URL into the form, not the date ranges/priority range/user fields the
form does control.
Fixed performAdvancedSearch() to start from the current URL's params
and only set/clear the ones this form actually represents, leaving
everything else untouched. Also fixed populateCurrentFilters() to
restore all of those fields, not just search/status — without that,
reopening the modal and submitting without touching anything would
now silently wipe date/priority/user filters that were active but
shown blank in the form (a new foot-gun the first fix alone would have
introduced).
Verified via jsdom: category/type/sort params not represented in the
form survive a search submission; page resets to 1; and reopening the
modal with an active created_from filter correctly restores it into
the form and preserves it on a no-op resubmit.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Lhz7pGMaoTfL5sdYS5XiKv
lt.api's fetch wrapper (_apiFetchAuth in base.js — the live
implementation lt.api.* resolves to) already resynced
window.CSRF_TOKEN from a 403 response's csrf_token field, but still
threw immediately — every caller saw a raw "Invalid CSRF token" error
on the FIRST attempt, with no transparent retry. Since CsrfMiddleware's
token lifetime (1h) is shorter than the session idle timeout (5h),
this was a routine, fully recoverable case (an hour of page
inactivity, or a write in another tab rotating the shared token), not
a real rejection.
After resyncing the token from a 403 body that carries one, now
retries the original request exactly once with the fresh token before
surfacing an error — transparent to the caller on the common case,
with a `retried` flag preventing more than one retry so a genuinely
broken session still fails cleanly instead of looping. Verified via
jsdom with a mocked fetch: a 403-then-succeeds sequence resolves
successfully with exactly 2 network calls and the correct final
token; a persistently-403 sequence still throws after exactly 2 calls
(no infinite retry).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Lhz7pGMaoTfL5sdYS5XiKv
The "load more attachments" append path used
grid.insertAdjacentHTML('beforeend', html), which the CI semgrep scan
flags as a blocking finding (detection of insertAdjacentHTML from a
non-constant string). The content was already fully escaped via
lt.escHtml() on every field, but switched to the same
temp-element + innerHTML + appendChild pattern used elsewhere to build
DOM from a generated HTML string, avoiding the flagged API without
changing behavior. Re-verified pagination append/remove behavior via
jsdom.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015nCxwFFsy8ouMWzn56rPVP
The P1/P2 SLA breach banner and progress bar were rendered server-side
at page load and never touched again. Changing a ticket's priority in
edit mode (P1->P3 or P3->P1) left the banner in a stale state —
showing/counting for a priority that no longer applied — until the
page was reloaded.
Moved the banner's render/update/teardown logic into a reusable
renderSlaBanner() in ticket.js (verified via jsdom against real
DOM: creates the banner for P1/P2, removes it when priority drops
below P2 or the ticket is closed, and re-creates it including the
already-breached state when priority is raised into P1/P2 range).
The priority-change handler now calls it after a successful update,
and the initial page load calls it once instead of relying on
duplicated server-rendered markup + inline script.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015nCxwFFsy8ouMWzn56rPVP
The sidebar's own Clear button cleared status/category/type/dates but
never search/priority/assigned_to, while the page-level "Clear All
Filters" button cleared a different subset. Neither control alone
reliably returned the dashboard to a fully unfiltered state. The
sidebar button now delegates to clearAllFilters() so both controls
always clear the same complete set of params.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015nCxwFFsy8ouMWzn56rPVP
insertMarkdownFormat, insertMarkdownText, toolbarList, toolbarHeading,
and toolbarQuote all set textarea.value = ... directly. Assigning
.value programmatically discards the browser's entire native undo
stack (vs. document.execCommand('insertText', ...), which preserves
it) — e.g. type a paragraph, click Bold, then Ctrl+Z undid the whole
paragraph instead of just the bold markup.
Added insertTextPreservingUndo(), which selects the exact range being
replaced and routes through execCommand('insertText', ...) — the same
mechanism real typing uses — falling back to the old direct assignment
(losing undo, matching prior behavior) only if execCommand is
unavailable or unsuccessful.
Verified with a jsdom harness that the fallback path (jsdom doesn't
implement execCommand, since native undo is a real-browser-only
feature untestable via jsdom) produces byte-identical resulting text
and cursor positions to the original implementation across all 5
toolbar functions, for both selected and cursor-only cases.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X
lt.sortable only wired dragstart/dragend/dragover/drop — the native
HTML5 Drag-and-Drop API. iOS Safari doesn't implement HTML5 DnD on
arbitrary elements at all, and mobile Chrome's support is poor, so
kanban card status-drag was effectively unusable via touch, despite
README's "Touch-friendly controls" claim.
Added touchstart/touchmove/touchend/touchcancel handling that mirrors
the existing mouse-based behavior: a small movement threshold (8px)
distinguishes a tap/scroll from drag intent, the dragged card is
repositioned via fixed positioning to follow the finger (reparented to
document.body to avoid clipping by an overflow:hidden ancestor), and
elementFromPoint resolves the hover target for the same
placeholder-insertion logic dragover already uses, including
cross-column moves via the shared group check.
touchmove/touchend/touchcancel are registered on document rather than
the sortable list itself: since touch events keep targeting their
touchstart element for the whole gesture regardless of DOM mutations,
and the dragged item gets reparented to document.body mid-drag, a
listener on the original list would stop receiving bubbled events
once that reparenting happens.
lt.sortable lives in base.js, the shared web_template copy used by
other LotusGuild apps (per its own header comment) — this fix should
be contributed upstream too, not just kept local to this repo.
Verified with a jsdom harness (stubbing getBoundingClientRect and
elementFromPoint against known layouts) covering: sub-threshold
movement not starting a drag, same-column reorder, cross-column move
with correct final DOM parent and order, and touchcancel cleanly
resetting state. This caught a real bug during development — an
earlier version listened on the list element for touchmove/touchend,
which silently stopped receiving events after the drag-start
reparenting.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X
The JS side claimed to "mirror the PHP crc32 % 4 logic" but actually
implemented a different rolling hash (classic String.hashCode()-style),
so the same display name could get different avatar colors depending
on whether a comment was server-rendered or client-rendered (new
comment, reply, watcher avatars, "Load more" pagination).
Added a real CRC-32 (IEEE 802.3/zlib polynomial, UTF-8 byte sequence)
to ticket.js and switched all three JS call sites (avatarColorClass,
watcher avatars, and buildCommentEl in TicketView.php) to use it,
verified to produce identical output to PHP's crc32() including for
non-ASCII names.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X
A user could set an end date before a start date, or priority_min >
priority_max, and the filter would be silently sent as an
unsatisfiable range with zero results and no explanation. Now swaps
min/max (and from/to) before building the query string when they're
in the wrong order.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X
Two small dead-code cleanups from a dashboard.js state-management
audit:
- sortTable(table, column) had zero callers — actual table sorting is
wired through lt.sortTable.init() via initTableSorting().
- setViewMode() wrote localStorage['ticketViewMode'], but nothing ever
read it back; the real view-mode restoration on page load reads
lt_activeTab_<path>, written separately by lt.tabs in base.js.
Both looked load-bearing but weren't, risking a future edit assuming
otherwise.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X
renderAttachments() decided whether to render an image thumbnail by
regex-matching the display filename extension, rather than the
finfo-verified mime_type the API already returns. A file whose real
type differs from its display name (e.g. a PDF a user named
photo.png) rendered a broken <img> instead of falling back to the
file-type icon.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X
quickAssign() wired lt.combobox.init() with an onSelect callback, but
combobox only supports the multi-select onChange(selected[]) contract
— onSelect is never invoked, so _quickAssignUserId stayed undefined no
matter what the user picked and Quick Assign always showed "Please
select a user from the list." Switched to lt.typeahead.init(), which
does support onSelect, matching the already-working Bulk Assign modal.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X
clearAllFilters() deleted the nonexistent date_from/date_to query
params. Every actual date filter (sidebar, Advanced Search, saved
filters, stat-card links) uses created_from/to, updated_from/to, and
closed_from/to, so clicking the button silently left any active date
range in place.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X
A modal can be dismissed four ways: the ✕ button, Cancel, a backdrop click, or
Escape. base.js handles the last two globally (a document click handler and
registerKey('escape', closeAllModals)), so the status-change modal — which wired
only the two buttons — never learned it had been dismissed. The status dropdown
kept displaying the new status even though update_ticket.php was never called,
so the ticket looked closed with no comment until a reload showed it still open.
The same gap left every dynamically-inserted modal in the DOM when dismissed
that way, so the next open inserted a duplicate id that shadowed the live one.
- base.js closeModal now dispatches a bubbling lt:modalclose event (synced to
web_template as bbec859), and _statusCommentModal treats it as "no comment".
- ticket.js reverts the dropdown on any dismissal, guarded against the re-entry
its own lt.modal.close() would otherwise cause.
- dashboard.js gains openModalWithDismiss() so all seven dynamic modals plus the
generic prompt modal tear down however they are dismissed.
Verified in headless chromium against all four dismissal routes plus a
confirm-with-comment control: 22/22. Against the pre-fix files the same test
fails 6 assertions — backdrop and Escape leave the dropdown on "Closed *" with
an orphaned overlay — so it reproduces the reported behaviour exactly.
Setting the textarea's .value programmatically does not fire an 'input' event,
so updatePreview() never ran and the preview kept showing the just-posted
comment's rendered markdown underneath an empty composer.
(This change was already present in the working tree at the start of the
session; committing it on its own rather than folding it into an unrelated fix.)
Bulk status changes previously bypassed the workflow entirely — the model
carried an explicit "admin-only escape hatch" note — so bulk edit could
drive tickets through transitions the designer forbids and skip comments
the designer requires.
BulkOperationsModel now applies the same rules as the single-ticket path:
- Transitions absent from status_transitions are refused per ticket and
reported with a reason, instead of being forced through.
- requires_comment is checked up front across the whole selection, so a
batch is rejected before any ticket is mutated rather than half-applied.
- The reason is persisted as a comment on each ticket changed, matching
what a single-ticket close records.
- Tickets already in the target status are a no-op success, not a failure.
requires_admin needs no extra check: api/bulk_operation.php already gates
the endpoint on admin.
Client: both bulk modals now collect a reason, the close path gets a real
modal instead of a bare confirm, and per-ticket skip reasons surface in
the result toast instead of a bare failure count.
Stored markdown comments rendered fine in the live preview (parses the raw
textarea value) but broke after refresh: the server template emitted the
comment text on an indented line, so the on-load renderer parsed
element.textContent with ~20 spaces of leading indentation. Markdown treats
4+ leading spaces as a code block, so the first line (e.g. a heading or
table row) was mis-parsed and blocks got wrapped in <p>, producing invalid
HTML that broke the page layout.
- markdown.js: trim the text before parseMarkdown in both on-load renderers
so template indentation can't be parsed as a leading code block.
- TicketView.php: emit the comment text inline (no surrounding whitespace)
so the element's textContent is exactly the stored markdown.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- base.js lt.api: resync window.CSRF_TOKEN from response bodies before
throwing on errors and attach err.data/err.status, so a desynced client
auto-recovers without a reload
- add lt.ticketStatus.submit: status changes that require a comment now
prompt, post the comment, and retry update_ticket with it; wired into
the ticket dropdown, dashboard quick-status, kanban drag-drop and the
1-4 keyboard shortcuts (bulk ops unchanged) — matches the new server
requires_comment enforcement
- base.js markdown.render: drop the unsafe marked/markdownit delegation;
always use the built-in XSS-safe renderer
- ticket.js: XHR upload sends the X-CSRF-Token header and resyncs the
token; use lt.escHtml instead of a re-inlined escape chain; @-mention
trigger requires a word boundary (no firing inside emails); idempotent,
anchor-safe highlightMentions
- base.js typeahead: discard out-of-order async results
- markdown.js: balanced table tbody/thead; ticket-ref linkification runs
after code extraction so #ids inside code aren't linked
- dashboard.js kanban: don't swallow the click after a drag
- keyboard-shortcuts.js: J/K skip hidden/skeleton rows; drop duplicate ?
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Medium:
- create_ticket_api.php: environment tags were parsed with explode('][') which
left brackets on the first/last tag so the whitelist never matched, dropping
the env tag from the dedup hash — a [production] and [staging] issue with
otherwise-identical components could collide onto one ticket. Use a
bracket-aware regex.
- CommentModel::getThreadedCommentsPaged only fetched DIRECT children of root
comments, so when pagination is active, nested replies at depth 2-3 vanished
from the thread. Expand replies level-by-level (bounded to depth 3).
- StatsModel::getTicketsByAssignee ignored the visibility filter the rest of the
stats apply, so a non-admin's "by assignee" widget counted (leaked) confidential
tickets. Thread the same filter through.
- watch_ticket.php GET path returned watch state / watcher names / count for any
ticket with no access check (the POST path checks it) — added canUserAccessTicket.
- dashboard.js kanban: every card rendered as P4 because the [class*="lt-p"]
selector never matched the lt-badge-p1 class and the fallback didn't strip "P".
Extract the digit directly.
Low:
- audit_log.php CSV: "Log ID" column was always blank ($log['log_id'] vs the real
audit_id column). Use audit_id.
- check_duplicates.php: the graceful-degradation try/catch only covered the throw
path; guard the false-return (non-exception mysqli) path too.
- notifications.php: owner-who-is-also-@mentioned got two notifications for one
comment; drop the duplicate comment row when a mention covers the same comment.
- dashboard.js hover preview rendered "PP1" (doubled prefix); strip the leading P.
- markdown.js: code/inline-code restore used string replace, so $&, $$, $`, $' in
user code were treated as replacement patterns; use a function replacer. Also
removed an unused loop var.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- SynapseHelper: memoize username->Matrix-ID lookups per-request (incl. negative
results) and add an overall time budget to resolveUsernames() plus a 2s connect
timeout, so notifying N watchers with a slow/unreachable Synapse can't stall the
request for N x 5s. (Chosen over async/queue per maintainer.)
- DependencyModel: fix cycle detection treating 'blocks' and 'blocked_by' as the
same edge direction. They are inverse relationships (single row each, no mirror
row), so the traversal now walks a unified precedence graph (blocks: ticket->
depends_on; blocked_by: depends_on->ticket) and wouldCreateCycle normalizes the
new edge's direction. Prevents both false-positive and missed cycles.
- CacheHelper: anchor prefix-delete to exact key boundaries (bare prefix or
prefix + '_' + md5) so delete('workflow') can't wipe a 'workflow_rules' cache.
- RateLimitMiddleware: hold an exclusive flock across the per-IP counter's
read-modify-write so concurrent requests can't both read N and write N+1
(undercounting past the limit). Fails open if the file can't be locked.
- dashboard.js: kanban status update now uses lt.api.post (per no-raw-fetch
convention) and reverts the card AND the optimistic column counts on failure
(the old raw-fetch catch left the card moved without reverting).
- BulkOperationsModel: document that bulk_status/bulk_close intentionally bypass
workflow transition validation (admin override, by design).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
XSS / security:
- markdown.js: sanitize footnote labels to a safe slug before using them in
id/href attributes. Labels are captured before the HTML-escape pass, so a
label like x"><img onerror=...> broke out → stored XSS (the earlier quote-
escape fix didn't cover this path). Verified neutralized.
- RateLimitMiddleware: only trust X-Forwarded-For / X-Real-IP when REMOTE_ADDR
is a configured trusted proxy, and use the rightmost (proxy-appended) entry.
Previously any client could rotate XFF to escape the per-IP rate limit.
- .env.example: document TRUSTED_PROXIES so fresh deploys aren't fail-open on
the Authelia forward-auth spoofing protection.
Correctness:
- notifications.php: my previous assigned-to LIKE fix anchored only on '}', so
BULK assignments (logged {"assigned_to":N,"bulk_operation_id":..}) produced
no "assigned to you" notification — now matches both '}' and ',' delimiters.
- notifications.php: implement the documented @mention notifications (query
action_type='mention' rows for the current user); they were never delivered.
- NotificationHelper::notifyWatchers: guard unchecked prepare() so a missing
ticket_watchers table can't fatal the request after its DB write committed.
- AuditLogModel::getTicketTimeline: JSON_UNQUOTE the extracted ticket_id so
comment events actually match (string vs JSON-number comparison never did).
- AuditLogModel/audit_log.php: CSV export no longer silently truncates to the
1000-row UI cap; uses a dedicated higher export limit.
- DashboardView: quick-preview drawer read .ticket-link from the title cell
(which has none), so the title was always blank — use the cell text.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Verified, high-confidence fixes from a project-wide review:
- markdown.js: escape " and ' in the HTML-escape step. User-controlled
image/link URLs and alt text were interpolated into "..." attributes
without quote escaping, allowing attribute breakout and injected event
handlers (stored XSS, only mitigated by CSP). Flagged independently by
two reviewers.
- cron/create_recurring_tickets.php & cron/cleanup_ratelimit.php: a
mangled crontab example inside the docblock contained */ which closed
the comment early, causing a fatal parse error — both cron jobs never
ran. Rewrote the docblocks without a literal */.
- update_ticket.php: validate visibility BEFORE the core DB write so an
invalid payload can't leave the ticket updated while the request reports
failure (which also skipped the audit delta and stats cache invalidation).
- watch_ticket.php: GET watcher_count was capped at 6 (count of a LIMIT 6
list); use an unbounded COUNT(*) so it matches the POST path.
- notifications.php: "assigned to me" LIKE pattern lacked a trailing
delimiter, so user 12 also matched 120/123/etc.; anchor with }.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- index.php: replace SQL string interpolation with concatenation + explicit
(int) casts for LIMIT/OFFSET; add nosemgrep for tainted-sql false positive
(WHERE clause built from hardcoded fragments with bound params only)
- api/upload_attachment.php: add realpath() path-traversal guard after mkdir
- api/user_avatar.php: make (int) cast explicit at cache-path construction;
add nosemgrep for tainted-filename false positive (integer-only input)
- assets/js/ticket.js: add nosemgrep for insertAdjacentHTML — all dynamic
content already escaped via lt.escHtml() before insertion
- .gitea/workflows/security.yml: exclude echoed-request rule globally —
all echo in API context is json_encode() output, not HTML; htmlentities()
fix semgrep suggests would corrupt JSON responses
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
parseInt(ticketId, 10) was stripping leading zeros before sending
to update_ticket.php. Switch to String(ticketId) for consistency
with all other ticket ID handling in the JS codebase.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- dashboard.js: use String(cb.value) instead of parseInt() in
getSelectedTicketIds() so zero-padded IDs like 000123456 are
preserved when sent to bulk_operation.php
- DashboardView.php: remove (int) cast on data-ticket-id attribute
for quick-status button; was stripping leading zeros
- TicketView.php: remove (int) cast on export URL ticket_id param
- update_ticket.php: preserve ticket_id as string via trim((string)...)
- add_comment.php: preserve ticket_id as string; validate with
ctype_digit instead of (int) cast so comments are stored with the
canonical zero-padded ID matching the tickets table
- export_tickets.php: validate singleId as string to avoid stripping
leading zeros in the export endpoint
- notifications.php: preserve ticket_id strings in URLs and ticket
ownership checks; index myTicketIds by both int and string forms
for robust lookup regardless of how audit_log stored the ID
- TicketController.php: fix inline dependency insert — column was
wrong (depends_on_ticket_id → depends_on_id) and bind types were
wrong ("iii" → "ssi"); feature was silently broken
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
comment-text divs with data-markdown were never getting the lt-markdown
class, so all scoped CSS (ul/ol/li bullets, mark, del, task items, etc.)
had no effect. Fixed in PHP template, JS comment builder, and
renderMarkdownComments().
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace broken regex-based list wrapping with placeholder approach:
each list item type (OLI/ULI/TDI/TTI) gets a unique tag, then consecutive
runs are wrapped in the correct <ol>/<ul> container. Mixed task + regular
items in the same list work correctly.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- markdown.js: add renderMarkdownComments() called on DOMContentLoaded to
process [data-markdown] elements that were never being rendered on page load
- CSP: allow https: in img-src so external images in markdown aren't blocked
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Support  syntax in comments/descriptions. Images are only
rendered for http/https URLs. Style via .md-image (max-width, border,
block display) consistent with existing .lt-markdown img rule.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Event delegation for toggle-sidebar was inside the isDashboard guard
so it could silently not register. Bind .lt-sidebar-toggle buttons
directly on DOMContentLoaded — simple and guaranteed to work.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
toggle-sidebar action was only in the DashboardView inline script,
not in dashboard.js where toggleSidebar() is defined. Move it into
the dashboard.js event delegation switch so it's guaranteed to fire.
Also fix beta webhook: was using a different secret than production
so Gitea pushes to development never triggered the beta deploy.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
JS was toggling .collapsed on the wrong element (dashboardSidebar div
instead of lt-sidebar aside), and the expand button was permanently
display:none. When collapsed, users had no way to re-expand.
- toggleSidebar now targets lt-sidebar (the aside)
- Toggle button flips ◀ ↔ ▶ to indicate state and serve as the expand button
- Collapsed CSS hides the body and label, centers the ▶ button in the strip
- Remove the dead sidebarExpandBtn element from HTML
- Persist and restore state correctly on page load
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Created Today tile: no longer limits to open statuses (count is all statuses)
- Closed Today tile: filters by closed_at range, not updated_at
- Add closed_from/closed_to support to TicketModel and DashboardController
- Add Created/Updated/Closed date range inputs to sidebar filter panel
- Apply button collects date inputs; Clear All removes them
- removeFilter handles date chip removal (clears both _from and _to)
- Active filter chips shown for date ranges
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
JS was querying .filter-group but the HTML uses .lt-filter-group,
so no checkboxes were ever collected and filters had no effect.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- add_comment.php: include user_id in response for avatar rendering
- ticket.js: add buildCommentElement() helper that matches server-rendered
comment structure (avatar, edit/delete buttons, textarea); use it in
addComment() and submitReply() so new comments show the avatar immediately
- AuditLogModel: logCommentCreate uses action_type='comment' not 'create'
- TicketView: formatAction handles entity_type='comment' with action_type='create'
for existing DB records; prevents "created this ticket" showing for comments
- update_ticket.php: remove owner/assignee restriction so any authenticated
team member can update ticket status and fields
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Line 1575 used .replace(/</g, '<').replace(/>/g, '>') to set
the comment-raw edit textarea content, missing '&' → '&'. Replaced
with lt.escHtml() which escapes all five special HTML characters (&, <,
>, ", ') consistently with the rest of the codebase.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The combobox modal used lt-combobox-list but lt.combobox looks for
lt-combobox-dropdown — it returned immediately, wiring nothing.
Replaced with lt.typeahead which is correct for single-select search:
- Filters users client-side as you type (minChars:1, debounced 150ms)
- Shows display_name (username) with highlight on match
- onSelect stores user ID and shows "✓ Name" confirmation below input
- Input auto-focuses when modal opens
- Enter key now selects first result even without arrow-key navigation
(same fix applied to lt.combobox Enter handler)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
jaredandClaude Sonnet 5