- Prune watchers when a ticket's visibility is tightened (#73)
- Re-check ticket visibility before surfacing in-app notifications (#48)
- Use lt.api instead of raw fetch() in notification bell (#57)
- Auto-retry once after CSRF token resync in lt.api (#86)
- Validate field_type against the allowed enum in custom field definitions (#50)
The setValue()/is_required/select-options half of this issue was
already fixed incidentally by #47's new api/ticket_custom_fields.php
endpoint. The remaining gap: createDefinition()/updateDefinition()
never validated field_type against the six values the schema's
enum() actually allows (text/textarea/select/checkbox/date/number), so
a malformed type could be stored via the admin API and break whatever
UI renders it later.
Added an ALLOWED_FIELD_TYPES allowlist check at the top of both
methods, returning the same ['success' => false, 'error' => ...] shape
they already use for a DB failure — api/custom_fields.php already
propagates that shape correctly with no changes needed there. Verified
against real MariaDB: an invalid field_type is rejected on both create
and update, while a valid one still succeeds.
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
layout_footer.php's loadNotifications() and "mark all read" handler
called fetch() directly instead of lt.api.*, violating the project's
own documented convention (README Dev Notes #20). api/bootstrap.php
rotates the CSRF token on every successful write and returns it in the
response's csrf_token field; lt.api.* reads that and updates
window.CSRF_TOKEN, but a raw fetch() never does — so after "mark all
read", the server had rotated its token but the client's cached one
was stale, causing the user's next write anywhere else in the app to
fail once with "Invalid CSRF token" before self-healing.
Replaced both fetch() calls with lt.api.get/post, which also drops the
now-redundant manual header/credentials boilerplate.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Lhz7pGMaoTfL5sdYS5XiKv
All four notification queries in api/notifications.php (assign,
comment, status-change, mention) were scoped purely by
created_by/assigned_to/ticket_watchers membership and historical
audit_log contents — never by canUserAccessTicket(). If a ticket's
visibility was later tightened, or a user's group/watcher access
revoked, a notification still surfaced in their bell dropdown,
disclosing the ticket's title and that activity occurred even though
opening the ticket itself would now be blocked.
Batch-fetches the tickets referenced by all candidate notifications
(via the existing getTicketsByIds()) and filters out any whose current
state canUserAccessTicket() would reject for the requesting user,
before formatting the response — so a notification for a ticket the
user can no longer see simply disappears rather than lingering as a
disclosure. Verified against real MariaDB with a running server: an
assignment notification is visible while the user is the assignee of
a public ticket, and disappears once the ticket is reassigned away and
made confidential.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Lhz7pGMaoTfL5sdYS5XiKv
TicketModel::updateVisibility() only updated the tickets row — it
never touched ticket_watchers. A user watching a public ticket that's
later made confidential/internal, and who isn't creator/assignee/
admin/in the new visibility_groups, kept receiving Matrix
notifications (title + redacted activity preview) about a ticket
canUserAccessTicket() would now reject them from opening directly.
After a successful visibility update, re-evaluates every current
watcher against the new visibility rules via the same
canUserAccessTicket() check the rest of the app uses, and removes any
who no longer qualify. Verified against real MariaDB: tightening to
confidential correctly drops watchers with no standing access while
keeping an admin watcher; tightening to internal with a specific group
correctly keeps a watcher in that group and drops one who isn't.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Lhz7pGMaoTfL5sdYS5XiKv
CustomFieldModel::setValue()/setValues()/getValuesForTicket() were
never called anywhere outside api/custom_fields.php's own admin CRUD
for field *definitions* — CreateTicketView.php never rendered custom
fields, TicketController::create() never collected or saved them, and
TicketView.php never displayed them. The whole feature (admin defines
fields at /admin/custom-fields, including marking them Required) was
config-only with zero consumer; is_required was enforced nowhere.
Added:
- api/ticket_custom_fields.php: new endpoint (bootstrap.php-based, so
any ticket editor can use it, not just admins) that saves values for
a ticket. Only considers fields applicable to the ticket's current
category (or category-less fields); enforces is_required, validates
select values against the field's configured options, and validates
number fields are numeric. Values for fields that don't apply are
silently ignored rather than persisted, so a value typed before a
category change can't linger as orphaned data.
- CreateTicketView.php: renders every active field definition (all
categories, since the ticket doesn't exist yet), grouped with a
data-custom-field-category attribute and toggled client-side as the
Category select changes, matching the existing visibility-groups
toggle pattern. Submitted as part of the same form.
- TicketController::create(): validates is_required server-side against
the fields applicable to the *submitted* category (not just whatever
was visible client-side) before creating the ticket, then persists
via CustomFieldModel::setValues() after a successful create.
- TicketView.php: new "Custom Fields" tab (only shown when the ticket's
category has applicable fields) rendering current values with a
single Save button, calling the new endpoint — a panel-plus-save
interaction rather than per-field inline auto-save, to keep scope
contained across 6 field types.
Verified against real MariaDB and a real running server: a required
field left blank is correctly rejected (both at ticket-creation time
and when editing an existing ticket) with no partial write; a valid
submission persists exactly the fields applicable to that ticket's
category; an invalid select value is rejected without touching
previously-saved values; and each rejection correctly returns a
rotated recovery csrf_token (initially missed on the validation-error
paths, since they used a plain echo/exit instead of the bootstrap.php
apiRespond() helper every other endpoint's error paths use for this).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Lhz7pGMaoTfL5sdYS5XiKv
- Fire notifications and fix audit_log shape for bulk status changes (#67, #74)
- Route hwmonDaemon's auto-reopen through Workflow Designer validation (#68)
create_ticket_api.php's dedup-reopen path wrote status = 'Open' via a
raw SQL UPDATE, completely bypassing TicketModel::updateTicket() and
WorkflowModel::isTransitionAllowed() — the one status-write path in
the app that never consulted the workflow engine at all. If an admin
configured the Workflow Designer to disallow a direct Closed->Open
transition, this automated path still forced it unconditionally.
Now checks isTransitionAllowed('Closed', 'Open', false) first (false
since this is an unattended system account, not admin-elevated). If
not allowed, falls back to any transition the Workflow Designer does
allow from Closed that requires neither a comment nor admin privilege
(both of which this unattended automation can't satisfy), and applies
it via TicketModel::updateTicket() instead of raw SQL. If no such
transition exists at all, the ticket is deliberately left Closed
(rather than forcing an unconfigured state) with a comment and audit
entry explaining why, so the recurrence is still visible to a human
without silently violating workflow rules.
Verified against real MariaDB across all three branches: direct
Closed->Open allowed (reopens to Open), disallowed but Closed->'In
Progress' available unattended (falls back correctly), and no usable
transition configured at all (ticket correctly stays Closed).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Lhz7pGMaoTfL5sdYS5XiKv
BulkOperationsModel's bulk_close/bulk_status paths had zero references
to NotificationHelper — the exact same status transition (e.g.
Open->Closed) silently produced no Matrix/watcher notification when
performed via bulk actions, while the single-ticket edit page and
Bearer API both notify on every status change. Separately, their
audit_log entries used a bare ['status' => 'Closed', ...] shape
instead of the {'status': {'from': X, 'to': Y}} shape every other
status-change path uses, which broke two downstream consumers:
TicketView.php's timeline fell back to a generic "updated this
ticket" instead of "updated status", and notifications.php's
$details['status']['from'] on a string produced a broken "? -> ?"
notification title.
Fixed the audit_log shape for both operation types, and added a
notification queue collected during the per-ticket loop and flushed
only after a successful commit (so atomic-mode rollback correctly
sends zero notifications, matching how nothing else about a rolled-
back batch takes effect either). Also fixed an incidental bug found
while matching this to the single-ticket path: update_ticket.php's
notifyWatchers() call never passed the ticket's visibility, silently
defaulting to 'public' and always including the shared notify list
even for confidential/internal tickets — the exact leak #71 fixed
elsewhere in NotificationHelper itself, just never reaching this
call site.
Verified against real MariaDB with a real local webhook-capturing
server: bulk_close correctly fires sendStatusChangeNotification() +
notifyWatchers() with the right old/new status and a redacted title
for a confidential ticket; audit_log rows show the correct {from,to}
shape; and an atomic-mode rollback (one ticket's transition invalid)
sends zero notifications and leaves both tickets unchanged.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Lhz7pGMaoTfL5sdYS5XiKv
AuthMiddleware::authenticate() only re-read Remote-User/Remote-Groups
(and thus is_admin, via UserModel::syncUserFromAuthelia) when
$_SESSION['user'] didn't exist yet. Once a session existed, every
subsequent request only checked the idle timer — never re-validating
against current Authelia/LLDAP state. An admin's group membership
revoked in LLDAP, or a logout at the Authelia proxy, left their
already-open session with full access for up to SESSION_TIMEOUT (5h
default), with no way to force early revocation short of clearing the
server-side session store.
Added PRIVILEGE_RESYNC_INTERVAL (default 5 min, matching
UserModel's own cache TTL) and a resyncPrivileges() check on every
already-authenticated request past that interval: re-reads the current
request's forward-auth headers (enforcing the trusted-proxy check
again, same as a fresh login), and either destroys the session and
redirects to re-auth if the user no longer has any required group, or
re-syncs is_admin/groups/display_name/email if they do. Best-effort if
this particular request doesn't carry forward-auth headers at all
(skips silently rather than force-logging out, retried next interval).
UserModel::syncUserFromAuthelia() has its own 5-minute in-process
cache keyed only by username (not by the groups being synced), so a
naive re-call during a resync would have kept returning the
pre-revocation cached result for up to 5 more minutes — invalidated
that cache entry immediately beforehand to guarantee a real re-sync.
Verified against real MariaDB across a fresh login, a same-interval
request confirming no premature resync, an admin-privilege-revocation
mid-session (is_admin flips to false in both session and DB, verified
via a direct query), and a full group-membership revocation (session
destroyed, redirected to re-auth, confirmed the request never reaches
past that point).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Lhz7pGMaoTfL5sdYS5XiKv
README documented ErrorHandler.php as a "global error/exception
handler", but ErrorHandler::init() had exactly one caller app-wide
(api/get_template.php). 13 endpoints never called
ini_set('display_errors', 0) at all, relying on the server's global
php.ini default, and index.php never registered any handler — a
genuine PHP fatal during a page render fell through to PHP's raw
default handling with no app-level 500 response, styled or otherwise.
Investigating the "13 endpoints" claim turned up that 9 of them
(assign_ticket.php, audit_log.php, check_duplicates.php,
get_comments.php, get_users.php, notifications.php, saved_filters.php,
user_preferences.php, watch_ticket.php) already require
api/bootstrap.php as their first statement, which itself calls
ini_set('display_errors', 0) — so they were never actually exposed;
the static grep just couldn't see through the require. The 3 that
were genuinely unprotected (bulk_operation.php, download_attachment.php,
health.php) are fixed here. ticket_dependencies.php already has its
own complete hand-rolled equivalent (shutdown handler, error handler,
exception handler, output-buffer aware) and was deliberately left
alone rather than risk double-registering handlers.
Rather than duplicate the fix 30+ times, wired ErrorHandler::init()
directly into api/bootstrap.php (covering all 9 files above at once)
and into each of the other endpoints' own ini_set/error_reporting
pair, replacing it in place — additive only: existing try/catch blocks
in every endpoint still handle what they already handled identically,
this only adds a safety net for genuinely uncaught fatals that fell
through everything else. Before doing this app-wide, removed
ErrorHandler::init()'s override of PHP's 'error_log' ini setting: it
redirected every error_log() call in the request to a fixed /tmp file,
which would have silently diverted logs away from wherever the server
is actually configured to send them the moment this got wired into
more than one endpoint. That override only existed to support
getRecentErrors(), which has zero callers app-wide.
For index.php (page views, not JSON), added an 'html' response mode to
ErrorHandler that renders a new views/error_500.php instead of a JSON
body. That view is deliberately self-contained (no layout_header.php,
no $GLOBALS/session/DB dependency) since a genuine fatal can happen
before config.php finishes loading or mid-session-start.
Verified: a real uncaught error with no prior output correctly
produces a clean JSON 500 (API mode) or the styled HTML page (page
mode) to the client while the full stack trace goes to error_log, not
the response; normal (non-fatal) requests through both a
bootstrap.php-based endpoint and index.php are byte-for-byte
unaffected. Full project phpcs pass is clean.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Lhz7pGMaoTfL5sdYS5XiKv
api/bootstrap.php's centralized CSRF handling echoes
CsrfMiddleware::getToken() on a 403 rejection specifically so
lt.api's client-side resync (assets/js/base.js) can recover once
window.CSRF_TOKEN goes stale (token expiry, or a write in another tab
rotating the shared session-scoped token). 12 endpoints duplicate
CsrfMiddleware::validateToken() inline instead of routing through
bootstrap.php, and their 403 body omitted csrf_token entirely —
custom_fields.php, clone_ticket.php, delete_comment.php,
delete_attachment.php, bulk_operation.php, generate_api_key.php,
manage_templates.php, manage_recurring.php, revoke_api_key.php,
manage_workflows.php, ticket_dependencies.php, and
upload_attachment.php.
Once a client's token drifted out of sync, the next write to any of
these 12 endpoints returned a 403 with no way to self-heal — every
subsequent write to any endpoint kept failing until a manual reload,
since the resync mechanism was only wired up on a minority of the
app's write surface. Took the minimal fix the issue names as
sufficient (add 'csrf_token' => CsrfMiddleware::getToken() to each
rejection body) rather than restructuring all 12 through bootstrap.php,
to avoid behavioral risk from rewiring each endpoint's differing
auth/bootstrapping. generate_api_key.php and revoke_api_key.php threw
a generic Exception for this case (swallowed into a plain error-message
response with no room for extra fields), so those two now short-circuit
with a direct JSON response instead, matching the other 10.
Verified end-to-end against real running endpoints with a real
session and real MariaDB: sent a wrong CSRF token to one endpoint of
each response shape (plain json_encode, ResponseHelper::error, and the
formerly exception-based path) and confirmed all three now return the
current valid csrf_token in the 403 body.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Lhz7pGMaoTfL5sdYS5XiKv
TRUSTED_PROXIES ships empty in .env.example, which disables
AuthMiddleware's reverse-proxy allowlist entirely — a fresh deployment
that doesn't explicitly set it has zero verification that
Remote-User/Remote-Groups headers actually came from the trusted
Authelia proxy. Anything that can reach the app directly (a
misconfigured firewall rule, an exposed container port, SSRF from
another internal service) can set Remote-User: admin and fully
impersonate any user with zero authentication. The enforcement logic
itself was already correct; this was purely a dangerous, easy-to-miss
default.
Added a boxed, unmissable warning around TRUSTED_PROXIES in
.env.example (previously just an inline comment easy to skim past),
added the same warning to README's setup instructions (which didn't
mention this variable at all), and added a Check 8 to api/health.php
that reports a 'warning' status when TRUSTED_PROXIES is empty, so a
deployment that forgets it doesn't go unnoticed after the fact.
Verified against real MariaDB via a running server: the health
endpoint correctly reports 'warning' when empty and 'ok' once set.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Lhz7pGMaoTfL5sdYS5XiKv
Both files opened their own raw new mysqli(...) connection instead of
using Database::getConnection(), missing the charset/timezone sync
every other connection gets. index.php's connection serves nearly all
non-API web traffic (dashboard, ticket view, ticket create) — any
NOW()/CURDATE()-based query through it used the DB server's default
session timezone instead of the app's configured TIMEZONE, and no
explicit utf8mb4 charset meant multi-byte characters typed into a
ticket via the non-JS POST fallback could get corrupted at write time.
create_ticket_api.php (the hwmonDaemon Bearer endpoint) had the same
timezone gap, risking created_at landing on the wrong 'day' relative
to every other ticket-creation path.
index.php's raw die("Connection failed: " . $conn->connect_error) also
leaked raw mysqli error text (host/user/failure reason) to any
unauthenticated visitor on a DB outage; it now logs via error_log()
and shows a generic message instead. create_ticket_api.php already
handled this correctly (JSON error + error_log, no leak) and needed no
behavior change there beyond the connection source.
Verified against real MariaDB: the new connection path reports the
configured -04:00 session time_zone and utf8mb4 charset, vs. SYSTEM
tz on the old raw-mysqli path; a simulated connection failure (bad
DB_NAME) confirmed only the generic message reaches the response body
while the raw driver error goes to error_log().
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Lhz7pGMaoTfL5sdYS5XiKv
- Redact ticket title for non-public tickets in create/status-change Matrix notifications (#46)
- Exclude shared notify list and redact title in notifyWatchers() for non-public tickets (#71)
- Redact ticket title for non-public tickets in assignment notifications (#72)
- Verify mentioned-user access before sending @mention notifications (#69)
sendMentionNotification(), called from add_comment.php, had no
visibility check at all — unlike sendCommentNotification()/
notifyWatchers() which redact the comment preview for non-public
tickets. Mentioning a user with zero standing access to a confidential
ticket (not creator/assignee/admin, not in visibility_groups) sent
them a Matrix DM with the full ticket title AND comment text — worse
than #46 since it's delivered directly to an individual rather than
diluted into a shared list.
add_comment.php now filters mentioned users through
canUserAccessTicket() before resolving Matrix IDs, skipping the
notification entirely for anyone without access (one of the two
options the issue names as acceptable). getMentionedUsers() needed to
start selecting is_admin and groups alongside user_id/username/
display_name, since canUserAccessTicket() requires them. Verified
against real MariaDB: a user mentioned on a confidential ticket they
don't own/aren't assigned to is correctly denied, a user in the
matching visibility_groups for an internal ticket is correctly
allowed, and the same user is correctly denied on a different internal
ticket whose group they're not in.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015nCxwFFsy8ouMWzn56rPVP
sendAssignmentNotification() had the same missing-visibility gap as
#46 in a separate function: assigning a user to a confidential/internal
ticket broadcast the ticket title to the shared Matrix notify list
unconditionally when MATRIX_NOTIFY_ASSIGNMENTS is enabled.
Threaded visibility through using the same redactedTitle() helper
added for #46, wired up from the already-fetched ticket row in
assign_ticket.php. The assignee is still DMed directly regardless,
since being assigned gives them standing access to the ticket — but
because notify_users is one shared payload, they see the same redacted
title as everyone else on it rather than a personalized one. Verified
end-to-end with a local HTTP server capturing the webhook payload for
both public and confidential tickets.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015nCxwFFsy8ouMWzn56rPVP
notifyWatchers() only redacted the comment/activity preview for
non-public tickets — the shared MATRIX_NOTIFY_USERS list was still
merged into notify_users unconditionally, and the ticket title was
never redacted at all. A status-change/comment notification for a
confidential ticket with watchers still broadcast that ticket's title
to the shared list, even though the function's own docblock intended
to protect non-public tickets from it.
For non-public tickets, the shared list is now excluded entirely
(only actual watchers are notified) and the title is redacted via the
same redactedTitle() helper added for #46. Verified against real
MariaDB with a real watcher row: for a confidential ticket, the
captured webhook payload has only the watcher's Matrix ID (no shared
list) and a redacted title; for the same ticket made public, the
shared list is included and the title passes through unchanged.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015nCxwFFsy8ouMWzn56rPVP
sendTicketNotification() and sendStatusChangeNotification() always sent
the ticket title to the shared MATRIX_NOTIFY_USERS list regardless of
visibility, unlike sendCommentNotification()/notifyWatchers() which
already redact the comment/activity preview for non-public tickets.
Creating or changing the status of a confidential ticket broadcast its
title to a shared Matrix room, defeating the point of the Confidential
visibility level.
Added a shared redactedTitle() helper and threaded visibility through
both functions (sendTicketNotification reads it from the existing
$ticketData['visibility'] key; sendStatusChangeNotification takes a new
optional parameter, wired up in both callers from the already-fetched
ticket row). Verified end-to-end with a local HTTP server capturing the
actual webhook payloads: public tickets pass the title through
unchanged, confidential/internal tickets get the redacted placeholder.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015nCxwFFsy8ouMWzn56rPVP
- Add missing rate limiting to create_ticket_api.php (#27)
- Fix visibility-group matching disagreement between filter and access check (#28)
- Replace illusory transaction wrapping in migrate.php with statement-level resume (#30)
- Fix ticket_watchers.ticket_id type mismatch and missing FK (#32)
ticket_watchers.ticket_id was int(11) while every other satellite
table (ticket_comments, ticket_attachments, ticket_dependencies,
custom_field_values) uses varchar(9)/varchar(10) matching
tickets.ticket_id, and it had no FK constraint at all — unlike every
other satellite table — so orphaned watcher rows could never be
caught by referential integrity.
Changed the column to varchar(9) with an ON DELETE CASCADE FK to
tickets, in both 000_baseline.sql and a new idempotent
004_fix_ticket_watchers_type.sql (which also deletes any pre-existing
orphaned watcher rows before adding the constraint, since orphans
would otherwise make the ADD CONSTRAINT fail). Updated
watch_ticket.php, NotificationHelper::notifyWatchers(), and
notifications.php's audit-log JOIN to bind/compare ticket_id as a
string instead of casting to int, including replacing a fragile
CAST(entity_id AS UNSIGNED) with a direct string comparison.
Verified against real MariaDB: applied 004 against a simulated
pre-fix deployment with one valid and one orphaned watcher row —
the orphan is removed, the column converts losslessly, the FK is
added, and the migration is idempotent on re-run. Confirmed
ON DELETE CASCADE actually removes watchers when their ticket is
deleted, that inserting a watcher for a nonexistent ticket now fails
with a real FK violation, and exercised the updated watch/unwatch and
status-change-notification query paths end-to-end against the fixed
schema.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015nCxwFFsy8ouMWzn56rPVP
migrate.php wrapped each migration file's statements in
begin_transaction()/rollback(), but MySQL DDL statements cause an
implicit commit — so a rollback couldn't actually undo earlier DDL
already executed within the same file. A migration failing partway
left the DB altered but unrecorded, and the next run retried the
whole file from statement 1, hitting "already exists" errors not on
the safe-to-ignore allowlist and permanently wedging the runner.
Removed the transaction wrapper (it only gave false confidence) and
added a migration_progress table that records the index of the last
successfully-executed statement in each file. A re-run after a
partial failure now resumes right after the last success instead of
re-executing already-applied DDL. Verified against real MariaDB with
a 4-statement migration where statement 3 fails: run 1 correctly
applies statements 1-2 and records progress at index 1; after fixing
the bad statement, run 2 resumes at statement 3, completes, and
clears the progress marker.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015nCxwFFsy8ouMWzn56rPVP
getVisibilityFilter() (dashboard list/stats) matched via
FIND_IN_SET(?, REPLACE(t.visibility_groups, ' ', '')) — stripping
spaces from the column but not from the bound group name — while
canUserAccessTicket() (single-ticket access) did a plain trim with no
space-stripping at all. For a group name containing a space (e.g. "IT
Support"), a member could open an internal ticket directly by URL but
never see it in their dashboard list or stats counts.
Now strips spaces from the bound parameter too, matching the column-
side normalization, so both paths agree. Verified against real
MariaDB: a ticket visible via canUserAccessTicket() for a
space-containing group is now also matched by getVisibilityFilter()'s
SQL, a wrong-group user is denied by both, and the plain no-space case
is unaffected.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015nCxwFFsy8ouMWzn56rPVP
Every other Bearer-key endpoint (ticket_status_api.php,
ticket_comment_api.php) calls RateLimitMiddleware::apply('api') before
opening a DB connection; create_ticket_api.php didn't, contradicting
README.md's claim that the whole Bearer API is rate-limited. A leaked
or guessed API key could hammer ticket creation unthrottled, each
insert also firing a Matrix webhook.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015nCxwFFsy8ouMWzn56rPVP
- Consolidate dashboard clear-filters controls to one shared function (#76)
- Make SLA priority-alert banner update live on priority change (#64)
- Add HTTP Range/partial-content support to attachment downloads (#99)
- Paginate attachment listing (#100)
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
AttachmentModel::getAttachments() had no LIMIT/OFFSET, so a ticket
with hundreds of attachments loaded and rendered every one of them in
a single API response and DOM grid, unbounded.
Added optional limit/offset to getAttachments(), matching the pattern
already used by CommentModel::getCommentsByTicketId(). The GET handler
in upload_attachment.php now accepts limit/offset (default 40, capped
at 100) and returns total/has_more alongside the page of attachments.
ticket.js's loadAttachments()/renderAttachments() now fetch and append
pages, showing a "Load more attachments (N remaining)" control when
more are available. Verified against real MariaDB with 12 attachments
across 3 pages of 5: no duplicates or gaps across pages, and the
legacy unlimited call (getAttachments($ticketId) with no
limit/offset) still returns everything unchanged.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015nCxwFFsy8ouMWzn56rPVP
download_attachment.php always streamed the entire file regardless of
any Range request header, and never advertised Accept-Ranges. Large
video/PDF attachments couldn't be scrubbed in-browser, and an
interrupted download had to restart from byte 0.
Now parses a single-range "bytes=start-end" (including open-ended and
suffix forms) request header and responds with 206 Partial Content and
a Content-Range header, seeking the file handle to the requested
offset; out-of-range requests get 416 with Content-Range: bytes
*/<size>. Verified against a real file served over a local PHP dev
server with curl for exact-range, open-ended, suffix, no-Range, and
out-of-bounds cases, confirming byte-identical output for each.
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
Fixes 15 tracked issues from the tinker_tickets tracker, all verified
against a local MariaDB instance and/or jsdom/manual test harnesses
where applicable: #75, #84, #102, #29, #53, #90, #60, #79, #61, #31,
#96, #41, #89, #59, #52, #42, #66, #40, #106, #54, #92, #97, #51, #91,
#101, #63, #55, #65, #107.
Also fixes a real, previously-undetected outage in .env.example:
parse_ini_file() could not parse the file as shipped (fragile '#'
comment handling plus an unquoted LDAP_BIND_DN value), meaning the
documented setup step of `cp .env.example .env` would have broken
every fresh deployment.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X
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
api/upload_attachment.php enforced a per-file size cap but nothing
bounded the total number of attachments on a single ticket or their
cumulative size over time — an authenticated low-privilege user could
slowly fill the uploads/ disk by attaching many files across tickets,
bounded only by the general rate limiter (which throttles request
rate, not storage volume).
Added MAX_ATTACHMENTS_PER_TICKET (50) and
MAX_TOTAL_ATTACHMENT_SIZE_PER_TICKET (100MB) config defaults, enforced
before move_uploaded_file() using AttachmentModel::getAttachmentCount()
and getTotalSizeForTicket() — both already existed in the model with
zero callers, apparently added for exactly this purpose but never
wired in.
Verified against a local MariaDB instance: with 3 existing 1MB
attachments and a 3-attachment cap, the count check correctly rejects
a 4th; with a 5MB total cap, a 2.5MB upload that would push the ticket
over the limit is correctly rejected while a small one that fits is
not.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X
Primary fix (#63): added the 5 env vars config.php reads and README
documents but .env.example never listed: MATRIX_DOMAIN,
SYNAPSE_ADMIN_URL, SYNAPSE_ADMIN_TOKEN, MATRIX_NOTIFY_COMMENTS, and
MATRIX_NOTIFY_ASSIGNMENTS. A deployer following only .env.example had
no indication these existed, silently missing watcher Matrix DMs and
comment/assignment notifications.
While verifying the fix by actually running .env.example through
parse_ini_file() (what config.php calls), found this file could not
be parsed at all — a real, currently-live outage for anyone following
its own first-line instruction ("Copy this file to .env and fill in
your values"):
1. PHP's ini parser treats "#" comments as fragile: punctuation like
parentheses or quotes inside a "#" comment can throw a syntax error
even though the line is meant to be inert. The file's header
comment itself (and 15+ other comment lines) tripped this. Switched
every comment to ";", which parse_ini_file treats as a true inert
comment regardless of content — verified with isolated repros of
both prefixes under all three INI_SCANNER_* modes.
2. LDAP_BIND_DN's example value contained unquoted "=" and commas,
violating the file's own documented quoting rule and causing a
second, independent parse failure. Quoted it (and the two other
comma-bearing LDAP DN values) to match the rule.
config.php has zero fallback for a parse failure — it die()s
immediately — so either bug alone would have taken down every fresh
deployment that didn't hand-edit the example file's comments first.
Verified end-to-end: copied .env.example to a real .env file
unmodified and ran it through config.php's exact parse_ini_file +
quote-stripping logic; it now parses cleanly with all 23 keys
(including the 5 new ones) and LDAP_BIND_DN resolves to the correct
unquoted DN string.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X
Two small findings from the same dashboard audit pass:
1. Charts rendered a bare empty frame with no message when the
filtered dataset was empty (fresh install, or a non-admin's
visibility-filtered ticket set happening to be zero). makeDonut/
makeBar now show a "No data for current filters" message in the
chart's place instead of silently doing nothing.
2. Stat cards had two independent, redundant click-handler
implementations. lt.statsFilter.init() (base.js, shared web_template
code) read each card's data-filter-key/data-filter-val attributes
and called window.lt_onStatFilter(key, val) on click — but that
global is never defined anywhere in this app, so it only toggled a
cosmetic .active class with no functional effect. The actual
navigation logic is the separate handler at ~line 1282 that ignores
those attributes entirely. Both fired on the same click with no
visible symptom, but the markup looked load-bearing and wasn't — a
trap for a future edit that touches one implementation assuming
it's the only one. Removed the dead lt.statsFilter.init() call and
the now-unused data-filter-key/data-filter-val attributes from this
app's DashboardView.php (left the shared lt.statsFilter module in
base.js itself untouched, since other LotusGuild apps consuming the
same shared template file may define their own lt_onStatFilter).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X
remember() had no protection against a slow cache-miss recomputation
overwriting a fresher write. If Request A started computing stats just
before a ticket mutation + invalidateCache(), and Request B started
just after (correctly computing fresh, post-mutation data), A could
finish (using stale pre-mutation data) after B and overwrite B's fresh
cache entry — extending staleness by up to another full TTL.
Added a per-prefix invalidation epoch: delete() bumps it, and
remember() snapshots it before running the callback and only writes
if the epoch hasn't changed since — otherwise a newer invalidation
happened mid-computation and the result being written is already
stale, so it's dropped (the caller still gets its own result; only the
cache write is skipped).
Verified with two real concurrent PHP processes racing against the
same cache key (a slow "Request A" callback vs. a fast "Request B"
that invalidates then recomputes): the cache ends up holding B's fresh
value, not A's late stale overwrite.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X
addDependency()'s "already exists" check only matched the exact
(ticket_id, depends_on_id, dependency_type) tuple. A user could add
"A blocks B" from ticket A's page, then separately add "B blocked_by
A" from ticket B's page — wouldCreateCycle() correctly found no cycle
(both normalize to the same precedence edge), so the insert was
allowed, creating two DB rows describing one real relationship (shown
twice on ticket B's page: once under Dependencies, once under
Dependents).
Added an inverse-relationship check before the insert: blocks/
blocked_by are inverses of each other, relates_to is its own inverse
(symmetric). duplicates has no defined inverse type in the schema, so
both directions remain independently insertable, which is correct —
"A duplicates B" and "B duplicates A" are distinct claims.
Verified against a local MariaDB instance: the exact repro from the
issue (A blocks B, then B blocked_by A) is now rejected, relates_to's
symmetric case is rejected in both directions, and duplicates in
either direction is unaffected.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X
api/upload_attachment.php did a raw move_uploaded_file() with zero
image processing. A photo attached from a phone retained embedded
EXIF, including GPS coordinates, and download_attachment.php streams
the file byte-for-byte back to any user with ticket visibility — for
an infrastructure company, this could leak a data center or office's
precise physical location through a routine ticket photo, especially
on Confidential-visibility tickets whose whole point is restricting
exactly this kind of detail.
Added stripImageMetadata(): decodes and re-encodes JPEG/PNG/GIF/WebP
uploads via GD, which drops EXIF chunks that aren't part of the pixel
data. Best-effort — leaves the file untouched on any failure (corrupt
image, unsupported format, GD unavailable, or an oversized decoded
pixel count guarding against a decompression-bomb-style crafted image)
rather than blocking the upload.
Verified with a real GPS-tagged JPEG (generated via piexif) and a
GD/PHP harness: GPS EXIF is gone after stripping, the image stays
valid and correctly sized, PNG alpha transparency is preserved, and
corrupt files / non-image MIME types are left byte-for-byte unchanged.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X
Dev Note #24 listed 7 callers; api/ticket_status_api.php (the Bearer
API's status-change endpoint) also correctly calls invalidateCache()
but wasn't in the list. Behavior was already correct — this is a pure
documentation completeness fix so the caller list stays an accurate
reference for future maintainers deciding whether a new mutating path
needs the same call.
Verified via grep -rl "invalidateCache" that these 8 files (plus
StatsModel.php itself, and an unrelated same-named method on
UserModel) are the complete set of real callers.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X
Zero callers anywhere in the app — confirmed via grep for
"OutputHelper::" across the whole codebase. Every view actually calls
htmlspecialchars() directly instead, which a prior audit confirmed is
done consistently, so escaping was never actually at risk. This was
just a misleading, unused class that README.md's file reference
implied was part of the app's active XSS-prevention story. Removed the
file and its README Project Structure entry.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X
config/requirements.php only checked PHP version and 6 extensions. A
deployment on a host with a low default memory_limit (e.g. shared-
hosting-style 128M) passed the startup requirements check cleanly and
only surfaced as a mysterious failure under real load — a large CSV
export, an oversized dashboard query on a big install.
Added min_memory_limit_mb (256) and min_max_execution_time (30s)
thresholds to config/requirements.php, checked as warnings (not hard
failures, since a low limit doesn't break every request) in both
scripts/check_requirements.php (CI) and api/health.php (production
monitoring). -1/0 (unlimited) always passes.
Verified the ini-size parsing and warning logic directly with
low/high/unlimited memory_limit and max_execution_time values.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X
bulk_operations.performed_by and ticket_templates.created_by had no
ON DELETE clause (defaulting to RESTRICT), unlike every other
user-reference FK in the schema (tickets.*, ticket_attachments,
ticket_dependencies, recurring_tickets, api_keys), which all use
SET NULL. Deleting a user who ever ran a bulk operation or created a
template hard-failed at the DB level instead of nulling the
reference, breaking the pattern used everywhere else.
performed_by was NOT NULL, so it had to become nullable to support
SET NULL, matching how every other SET NULL column is defined.
- Fixed 000_baseline.sql for fresh installs.
- Added 003_fk_on_delete_set_null.sql for existing deployments.
Verified against a local MariaDB instance: reproduced the old RESTRICT
schema, ran the migration (twice, for idempotency), then confirmed
deleting a user with rows in both tables now nulls the references
instead of failing.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X
deleteOldLogs() ran a single unbounded DELETE. created_at is indexed
so row selection itself is cheap, but on a large qualifying set (first
run after enabling/changing AUDIT_LOG_RETENTION_DAYS, or after the
cron silently missed runs) an unbounded single-statement DELETE holds
row locks for the full duration — risking contention with the frequent
concurrent INSERTs the audit log receives from live traffic. Now
deletes in batches of 1000 (parameterized), looping until nothing
qualifies.
Verified against a local MariaDB instance with a batch size of 10
forcing multiple loop iterations: deleted exactly the stale rows,
left recent rows untouched, correct total count returned.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X
api/get_users.php returned every user's user_id/username/display_name
to any authenticated session with no pagination or limit — needed for
mention/assignment typeahead, but a blanket enumeration a compromised
low-privilege session could scrape in one call. Added a LIMIT 500;
every caller already only uses this for typeahead/dropdown filtering,
never a literal full roster, so this doesn't change behavior for any
real deployment size while bounding the response.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X
The last_activity subquery had no WHERE clause on the report's
date-range filter, so it always showed true all-time last activity
even when the page was filtered to e.g. "last 7 days" — inconsistent
with every other column on the same report. Added the same
DATE(created_at) BETWEEN ? AND ? clause used by the report's other
subqueries.
Verified against a local MariaDB instance: an out-of-range audit_log
row is correctly excluded from last_activity once the filter is
applied.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X
setInterval(loadNotifications, 60000) ran unconditionally regardless
of tab visibility, and failures retried at the same fixed 60s cadence
forever. Now skips polling while document.hidden, resumes immediately
via visibilitychange when the tab regains focus, and backs off
exponentially (capped at 5 min) on repeated fetch failures, resetting
to the normal 60s cadence on the next success.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X
toggleActive() flipped is_active without touching next_run_at. If a
schedule was disabled while next_run_at was still in the future, then
re-enabled after that date had passed, the next cron tick saw
next_run_at <= NOW() and fired immediately — surprising for an admin
expecting a re-enabled "daily" schedule to wait until its next natural
occurrence. Now recomputes next_run_at from the current time when
transitioning to active, matching what a fresh schedule creation would
produce; disabling is unchanged.
Verified against a local MariaDB instance: re-enabling a schedule
whose next_run_at was in 2020 recomputed it to tomorrow at the
scheduled time; disabling leaves next_run_at untouched.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X
README Dev Note #12 mandates utf8mb4_general_ci for new tables, but
these two tables were created with utf8mb4_unicode_ci in the baseline
schema — inconsistent with every other table, and a future join or
comparison against a general_ci column would need explicit COLLATE
casts or hit "Illegal mix of collations" errors.
- Fixed 000_baseline.sql so a fresh install matches the convention
directly.
- Added 002_fix_collation_consistency.sql for existing deployments.
MariaDB silently drops the inline CHECK (json_valid(...)) constraint
on saved_filters.filter_criteria when that column is MODIFYed (found
by actually running this against a local MariaDB instance), so the
migration explicitly re-adds it after the collation conversion.
Verified against a local MariaDB 10.11: baseline applies cleanly,
migration is idempotent (safe to run twice), and the json_valid CHECK
is still enforced afterward.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X
syncUserFromAuthelia() did a plain check-then-insert with no
transaction, so two simultaneous first-visit requests for the same
brand-new user (e.g. two tabs opened right after SSO login) could
race: the second INSERT hits users.username's UNIQUE KEY, which
mysqli throws on (uncaught, PHP 8.1+ default report mode) rather than
returning false. Switched to INSERT ... ON DUPLICATE KEY UPDATE
followed by a re-fetch by username, so the losing request updates the
winner's row instead of throwing.
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
Zero callers anywhere in the codebase — superseded by claimForRun(),
which the cron script actually uses and which additionally guards
against the double-fire race between concurrent cron invocations that
this method lacked. Removing it so a future reuse doesn't silently
reintroduce that race.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X
CreateTicketView.php was the one remaining spot using the native
confirm() dialog, violating README Dev Note #21. Split loadTemplate()
into a confirm check + applyTemplate(), routed through the project's
styled showConfirmModal(), matching every other destructive-action
confirmation in the app.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X
gotoFilter() built a brand-new URLSearchParams containing only the
clicked chart segment's filter keys, discarding every other active
filter (search text, date range, saved-filter selection, etc.) on
navigation. Now merges the segment's filter into the current
location.search, same fix approach already applied to #22.
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
The 'update' notification formatter unconditionally read
details['status']['from']/['to'], so any title/priority/description/
category/type/visibility-only edit fell through to '?' on both sides
and produced a broken "changed status on #123: ? → ?" title regardless
of what actually changed. Now it branches on the delta shape actually
present: the flat {field, from, to} shape used for visibility changes,
then each per-field {from, to} delta in priority order, falling back
to a generic "updated ticket" message only if none match.
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
add_comment.php computed a trimmed copy of comment_text only to check
for empty input, then passed the original untrimmed $data through to
CommentModel::addComment(), so any leading/trailing whitespace the
user typed (or pasted) was written to ticket_comments.comment_text as-is.
update_comment.php already trims before saving edits, so a comment
could pass through this endpoint once with untrimmed text (creation)
and be silently corrected the moment it was next edited — inconsistent
storage that, combined with the markdown parser's line-anchored regexes
(headings, tables, lists all match on ^), could make a markdown-enabled
comment mis-render after a reload depending on whether its first line
carried leading whitespace.
Also trims in the "Load more comments" pagination re-render path in
TicketView.php, matching the two on-load renderers in markdown.js so
all three code paths that call parseMarkdown() on stored comment text
treat leading whitespace consistently.
Closes #18
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
All three charts (priority donut, status donut, category bar) now navigate to
the same URL filters the stat cards already use, with a pointer cursor on
hover, a title hint, and "click to filter" in the tooltip.
The status each click applies is explicit rather than left to the default. With
no `status` param the controller falls back to the viewer's
default_status_filters preference, which can be anything, so the list would not
necessarily match what the chart counted. StatsModel builds by_priority and
by_category with `status != 'Closed'` while by_status spans every status, so
only the priority and category charts pin the open set; the status chart filters
on the clicked status alone (which is how clicking "Closed" works at all).
Verified two ways:
- 17/17 in headless chromium, driving the real chart script from this view with
the Chart constructor stubbed, asserting the exact query each click produces
and that a click hitting no segment navigates nowhere.
- Against the live database, every segment's count equals the number of tickets
its filter returns — 12/12 across all three charts — so the list you land on
matches the number you clicked.
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.)
.ticket-preview-popup used var(--lt-surface), which is not defined anywhere, so
the background always fell through to the hardcoded #0a0e14. In light mode that
left a near-black panel — and since the rule set no `color`, the inherited
near-black body text was effectively invisible on it. The border was hardcoded
neon green and the shadow a heavy rgba(0,0,0,0.5).
Now uses --bg-card / --text-primary / --accent-green-border / --shadow-color,
and .preview-id uses --accent-cyan instead of the undefined --lt-cyan.
base.css gains the two tokens the light theme was missing (--accent-green-border
and --shadow-color), synced from web_template 0d633bd.
Verified with computed styles in headless chromium: light body-text contrast on
the panel goes from invisible to 17.7:1, dark stays at 13.2:1, and the ID accent
clears 3:1 in both themes.
Two separate causes, both light-mode-only:
1. base.css `.lt-select` sets `color-scheme: dark` on the element itself, which
outranks the `color-scheme: light` the light theme sets on <html>, so the
native dropdown popup kept dark chrome. The option list is also hardcoded
#0d1117/#c9d1d9 with no light override. Fixed with light overrides for both
(synced from web_template, where the same fix landed as 378a8cd).
2. ticket.css coloured the status select with var(--lt-success), --lt-amber,
--lt-cyan and --lt-danger — none of which are defined anywhere in the
project, so all four always fell through to hardcoded neon fallbacks. Now
uses the --accent-* tokens, which carry the same hues and are redefined for
light mode. The selectors also lead with .lt-select: at two classes they lost
to base.css's `html[data-theme="light"] .lt-select` (0,2,1) and every status
was repainted near-black in light mode.
Verified with computed styles in headless chromium — all four statuses in both
themes (8/8), plus the popup colour-scheme and option colours.
Found while verifying #21 against the live schema: the model writes
'completed_with_errors' (21 chars) when a bulk operation finishes with
per-ticket failures, but bulk_operations.status was varchar(20), so the
write failed with "Data too long for column 'status'".
This was latent — bulk status changes previously forced every transition
through, so failed was always 0. Now that they honour the Workflow
Designer, a partially-skipped batch is a normal outcome and hits it.
- migrations/001 widens the column to varchar(32) (idempotent).
- The baseline is updated to match, for fresh installs.
- The bookkeeping UPDATE is wrapped in a try/catch: it runs after the
ticket changes are committed, so an instance deployed ahead of its
migrations must not turn a completed operation into an error response.
Verified against the live database with a disposable-ticket harness:
comment-required rejection changes nothing, undefined transitions are
refused per ticket with a reason, allowed transitions still work, mixed
batches apply the valid half, and an already-Closed ticket is a no-op.
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.
- README: Bearer API table (list/read/comment/status), scope explanation,
and the new endpoints in the API Endpoints table.
- /admin/api-keys API Usage section: scopes note + copy-paste cURL examples
for create, list/triage, read-one, comment, and close (uses APP_DOMAIN).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Extends the Bearer-key API beyond create-only (all rate-limited, scope-
enforced, per-key-label attribution):
- GET /api/tickets_api.php: triage the queue (status/priority/host title
match + pagination) or read one ticket + its comments. read scope.
- POST /api/ticket_comment_api.php: post a comment as the key (user_name =
key name, linked to the key owner). read_write scope.
- POST /api/ticket_status_api.php: change/close status with workflow
validation + requires_comment; posts the close reason in the same call,
fires the Matrix status notification, invalidates stats. read_write scope.
Reuses TicketModel/CommentModel/WorkflowModel/NotificationHelper; a read
key cannot mutate. Reachability requires the reverse-proxy Authelia bypass
(handled separately).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Foundation for extending the Bearer API beyond create-only:
- api_keys gains a scope column (read | read_write); baseline schema updated
and the column applied to the live DB. Existing keys default to
read_write so the hwmon create key keeps working.
- ApiKeyModel: createKey() takes a validated scope; validateKey() always
surfaces scope (defaults read_write); getAllKeys() is paginated
({keys,total,page,perPage}, key_hash stripped).
- ApiKeyAuth: expose getKeyContext() (scope/key_name/created_by/api_key_id)
and requireScope() (403 on insufficient scope); existing return values
unchanged.
- create_ticket_api.php: require read_write scope (a read key can't create).
- Admin /admin/api-keys: scope selector on the create form, a scope column,
and pagination (revoked keys were stacking up).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
CI has been red since the CSRF-drift changes landed a trailing space on the
'success' => false line in these two endpoints, which blocks the deploy job
(and therefore beta/prod). No logic change.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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>
The hosts were upgraded to PHP 8.4, where mysqli::ping() is deprecated
(auto-reconnect was removed in 8.2). Database::getConnection() called it on
every reused connection, and api/ticket_dependencies.php's custom error
handler treated the deprecation as a fatal 500 ('A server error occurred'),
breaking the ticket Dependencies tab.
- Database.php: remove the redundant ping()/reconnect check (connection is
request-scoped; no liveness check needed on PHP 8.2+).
- ticket_dependencies.php: only abort on genuine errors; log notices/
warnings/deprecations and continue, so a future deprecation can't 500 it.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- migrations/000_baseline.sql: full schema baseline captured from prod
(validated on a throwaway DB: 17 tables/17 FKs), so the schema is
reproducible for fresh installs / disaster recovery
- create_recurring_tickets cron: send the Matrix ticket-created
notification and invalidate the stats cache like the other create paths
- create_ticket_api.php + TicketController::create: invalidate the stats
cache on create/escalate/reopen so dashboard counts aren't stale
- scripts/cleanup_orphan_uploads.php: restored, made safe (24h mtime
grace, 9-digit-dir only, skips avatars/symlinks, matches the unique
filename column, --dry-run)
- cron/cleanup_audit_log.php: enforce the configured audit-log retention
(deleteOldLogs was implemented but never called)
- README: correct CSRF-rotation, hwmon dedup (no 24h window), SLA (no P3),
stats-cache callers, and the project structure/endpoint listing
- .env.example: document TRUSTED_PROXIES fail-open risk and .env quoting
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Consolidate the duplicated command palette to a single overlay + init in
the footer; fix New Ticket to route to /ticket/create (was a 404 /create);
keep the CSP nonce and all commands
- TicketController create(): trim title, require a non-empty description,
and honor the posted status (validated against the canonical list) instead
of silently discarding it
- UserActivityView: 'Active Users' counts only users active in the selected
range, not every registered user
- layout_footer/DashboardView: local esc() now escapes quotes so values used
in HTML attributes can't break out
- TicketView: comments tab badge shows the true total, not just page one
- layout_header: gate the 'View activity log' link behind the admin flag
- index.php: validate /admin/user-activity date params; anchor the legacy
/ticket.php route; align the audit action-type whitelist with the dropdown
- ApiKeysView: correct the external API sample to /create_ticket_api.php
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>
- create_ticket_api.php: remove the wrong CREATE TABLE stub that broke a
fresh DB; generate collision-safe ticket_ids so a genuine id collision
isn't misreported as a duplicate and a hw alert dropped; stop leaking
raw DB errors; correct a reopen comment that falsely claimed refreshed
sensor data
- manage_recurring.php: fix next-run so create/edit no longer skips the
current period (monthly day-of-month this month, daily today if time
not passed, correct ISO weekday, month-length clamp); only recompute
on schedule changes to avoid double-fire
- export_tickets.php, audit_log.php: neutralize CSV formula injection
- revoke_api_key.php, generate_api_key.php: correct HTTP status codes and
stop the catch clobbering specific 4xx codes
- health.php: stop leaking PHP version / extension names / paths to
unauthenticated callers
- watch_ticket.php: define $data before use
- manage_templates/recurring/custom_fields: add audit logging for CRUD;
add recurring_ticket + custom_field to the audit entity whitelist
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- ticket_dependencies.php: pass current user id/groups/is_admin into the
visibility-filtered DependencyModel methods; drop (int) casts that
stripped leading zeros from varchar ticket_ids
- update_ticket.php: authorize visibility changes (admin or creator only);
enforce requires_comment transitions server-side (400 + requires_comment
flag so the client can prompt-and-retry); return proper 401/400/403
- add_comment.php: take commenter name from the session not the client
(anti-spoofing); validate parent_comment_id belongs to the ticket;
reject empty comments; pass ticket visibility to notifications so
non-public comment bodies aren't leaked
- add_comment/update_comment/bulk_operation: validate CSRF for all
state-changing methods, not just POST
- bootstrap.php: return the current CSRF token on rejection and never
rotate it on a rejected request, so a desynced client can auto-recover
- correct auth->401 and validation->400 status codes across these endpoints
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Database.php: pin MySQL session time_zone to the configured named zone
(mysql.time_zone tables now loaded on the DB) with a fixed-offset
fallback, so NOW()/TIMESTAMP and PHP agree regardless of the DB server's
SYSTEM tz. Best-effort, never fatals the connection.
- NotificationHelper: redact comment-body previews for internal/
confidential tickets in sendCommentNotification and notifyWatchers so
they are not leaked to the shared Matrix notify list (new $visibility
param; callers wired in the API batch).
- config.php: die with a clear error if parse_ini_file fails instead of
silently falling back to insecure defaults (empty DB pass / proxies).
- CacheHelper: create cache dir 0700 and cache files 0600 so other local
users cannot read or poison security-relevant cached data.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- CustomFieldModel: assign ?? fallbacks to variables before bind_param
(by-reference args cannot be ?? expressions; fatal on PHP 8.2, custom
fields were uncreatable/uneditable)
- RecurringTicketModel::create: fix swapped bind type for schedule_type
(enum bound as int coerced 'daily' to 0, breaking the cron)
- TicketModel/CommentModel: bind varchar ticket_id as string not int so
the unique index is usable and leading-zero IDs match; ticket_watchers
(int column) left as integer
- TicketModel::deleteTicket: delete from custom_field_values (real table)
not the nonexistent ticket_custom_fields
- TicketModel search: honor literal '0'; never emit AGAINST('*') on
all-special-char input (fall back to LIKE)
- TicketModel::updateTicket: disambiguate not-found vs no-op vs genuine
optimistic-lock conflict on zero affected rows
- WorkflowModel: do not cache transitions/statuses on DB failure (a
transient error no longer blocks all status changes for the TTL)
- DependencyModel: filter linked tickets by visibility (new optional user
context params) to stop confidential metadata leaking via dependencies
- BulkOperationsModel: validate status/priority/assignee before mutating
- AuditLogModel: gate getClientIP forwarded headers on trusted proxies;
add missing action/entity types so audit-log filters work
- WorkflowModel: add transitionRequiresComment() accessor for enforcement
- CommentModel: stop leaking raw DB errors to clients (log instead)
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The priority-escalation and recurrence comments embedded the full ASCII
alert description in a code block, producing a wall-of-text comment every
time. Since the ticket DESCRIPTION is already refreshed with the current
sensor data on each update, the comment only needs to record the event:
- Escalation: short note with from/to priority labels + a brief reason
("more severe condition reported, needs faster attention; see description").
- Recurrence: short reopened note pointing at the refreshed description.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
semgrep's github-actions-mutable-action-tag rule (now running, after the
pip install was fixed) flags actions/checkout@v3 as a mutable tag that
could be repointed upstream (supply-chain risk). Pin all four uses to the
SHA the v3 tag currently resolves to (v3.6.0), preserving behavior.
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>
jared
