Commit Graph
551 Commits
Author SHA1 Message Date
jaredandClaude Opus 5.5 dad066cb32 Merge development into main: MCP server documentation (#111)
Lint / PHP (phpcs PSR-12) (push) Successful in 21s
Lint / JS (eslint) (push) Successful in 13s
Lint / PHP requirements (version + extensions) (push) Successful in 49s
Lint / Notify on failure (push) Skipped
Security / PHP Security (semgrep) (push) Successful in 1m55s
Lint / Deploy (push) Successful in 2s
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X
deploy-2026.09.24-258
2026-09-24 19:26:42 -04:00
jaredandClaude Opus 5.5 9c633aa09b Document the MCP server: connecting Claude Code, tools, architecture (#111)
Lint / PHP (phpcs PSR-12) (push) Successful in 42s
Lint / JS (eslint) (push) Successful in 16s
Lint / PHP requirements (version + extensions) (push) Successful in 42s
Lint / Notify on failure (push) Skipped
Security / PHP Security (semgrep) (push) Successful in 1m46s
Lint / Deploy (push) Successful in 2s
Adds a README section on connecting Claude Code to /mcp (the pre-registered
client id + fixed callback port Authelia requires, `claude mcp login`
incl. --no-browser for headless hosts), the six tools and their scopes,
and how the pieces fit together: Authelia as the authorization server
(and the 4.39.21/22 RFC 8707 bug to avoid), token validation incl. the
per-server audience, Composer being MCP-only, and the proxy exemptions.
Also updates the project-structure tree (mcp/, services/,
ApiTicketController, composer files, migrations 005-007) and notes that
uploads/ is only served via PHP.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X
2026-09-24 19:26:40 -04:00
jaredandClaude Opus 5.5 9bbe4ef9a0 Merge development into main: OAuth-protected remote MCP server (#111)
Lint / PHP (phpcs PSR-12) (push) Successful in 19s
Lint / JS (eslint) (push) Successful in 8s
Lint / PHP requirements (version + extensions) (push) Successful in 22s
Lint / Notify on failure (push) Skipped
Security / PHP Security (semgrep) (push) Successful in 1m34s
Lint / Deploy (push) Successful in 3s
Adds /mcp, a remote MCP server (official MCP PHP SDK, pinned 0.8.1)
protected by Authelia-issued OAuth access tokens, so Claude Code can work
with Tinker Tickets as the signed-in user: search_tickets, get_ticket,
create_ticket, add_comment, update_status, assign_ticket. Tools run the
same code paths as the web UI (ApiTicketController moved to its own
file; CommentService, AssignmentService and TicketCreationService
extracted from their web endpoints). Composer is introduced for the MCP
endpoint only.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X
deploy-2026.09.24-254
2026-09-24 19:21:30 -04:00
jaredandClaude Opus 5.5 17be55bf8f Add MCP write tools: create_ticket, add_comment, update_status, assign_ticket (#111, phase 5)
Lint / PHP (phpcs PSR-12) (push) Successful in 20s
Lint / JS (eslint) (push) Successful in 8s
Lint / PHP requirements (version + extensions) (push) Successful in 20s
Lint / Notify on failure (push) Skipped
Security / PHP Security (semgrep) (push) Successful in 1m5s
Lint / Deploy (push) Successful in 2s
Each tool is a thin adapter over the same code path the web UI uses
(TicketCreationService, CommentService, ApiTicketController,
AssignmentService), run as the signed-in user, so permissions, Workflow
Designer rules, audit entries, notifications and stats-cache
invalidation are identical to doing the same thing in the browser.

- Registered in ToolCatalog and listed in its WRITE_TOOLS, so
  ToolScopeMiddleware requires tickets:write for them.
- Annotated readOnlyHint=false / destructiveHint=false (nothing deletes).
- Input the web form constrains with dropdowns (priority 1-5, visibility,
  status) is validated in the tools. Assignees are "me", a username or
  "unassigned".
- A ticket the user can't see reads as "not found" (never "access
  denied"), consistent with get_ticket.
- update_status turns requires_comment into an actionable error and
  invalidates the stats cache like api/update_ticket.php does.

Verified locally through the real pipeline (only JWT validation stubbed)
against MariaDB with seeded workflow transitions: 30/30 checks, including
a read-only token getting 403 insufficient_scope on create_ticket with
nothing written; create/comment/status/assign attributed and
audit-logged as the user; @mentions; internal visibility needing groups
and staying hidden from non-members; an invisible confidential ticket
not found for comment/status; requires_comment enforced, and closing
with a reason persisted in one transaction; transitions outside the
workflow refused; the admin/creator/assignee rule for assigning.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X
2026-09-24 19:13:41 -04:00
jaredandClaude Opus 5.5 f206bb5889 Extract ticket creation from TicketController into TicketCreationService (#111)
Required-field and required-custom-field validation, createTicket
(which enforces visibility rules), the audit log, stats-cache
invalidation, custom field values, the optional 'duplicates' link and
the new-ticket notification move into services/TicketCreationService.php
with the same order and error messages, so the MCP create_ticket tool
runs one code path with the web form. TicketController::create keeps
CSRF, the redirect, and re-rendering the form on error (the view never
read the removed locals). The service also accepts visibility_groups as
a string for API callers; the controller still passes only the form's
array, so web behaviour is unchanged.

Verified the real web form over HTTP (logged in via Remote-User, real
CSRF token): a valid submit redirects to the new ticket, created and
audit-logged as the user; a blank description re-renders the form with
'Description is required' and creates nothing.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X
2026-09-24 19:13:31 -04:00
jaredandClaude Opus 5.5 9e462f7f00 Extract ticket assignment from assign_ticket.php into AssignmentService (#111)
The access check, the admin/creator/current-assignee permission rule,
unassign/assign, the audit log, the optional Matrix assignment
notification and the stats-cache invalidation move into
services/AssignmentService.php for reuse by the MCP assign_ticket tool.
Error messages and status codes are unchanged.

One deliberate difference: assign_ticket.php's early error responses
(400/403/404) used a bare echo and so omitted the CSRF token that
bootstrap had just rotated. Every response now goes through
apiRespond(), which includes it. The front end already resyncs its
token from any response body, so this is compatible, and a failed
assign can no longer leave the page holding a stale token.

Verified over real HTTP: the assignee can reassign (200); a user who can
see the ticket but isn't admin/creator/assignee gets 403 'Permission
denied'.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X
2026-09-24 19:13:31 -04:00
jaredandClaude Opus 5.5 d5832fb58a Extract comment creation from add_comment.php into CommentService (#111)
Validation, the ticket access check, reply-parent validation, @mention
extraction (audit-logged, notified only to mentioned users who can see
the ticket), and comment/watcher notifications move into
services/CommentService.php, so the MCP add_comment tool runs one code
path with the web UI. add_comment.php keeps session, CSRF, JSON parsing
and response codes. Error messages and status codes are unchanged; the
extracted body diffs against the original only where each 'emit error
and exit' became a 'return [..., http_status]'.

Verified the web endpoint over real HTTP: a comment is trimmed, saved
with its @mention and the rotated CSRF token returned; a confidential
ticket the user can't see still gets 403 'Access denied'; empty text
still gets 400.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X
2026-09-24 19:13:31 -04:00
jaredandClaude Opus 5.5 6ce3380d6a Move ApiTicketController out of update_ticket.php into its own file (#111)
The partial-update controller (status transitions incl. requires_comment
with the comment in the same transaction, field edits, visibility,
audit delta, status-change notifications) was defined inline inside
api/update_ticket.php, so nothing else could reuse it. Moved it verbatim
to controllers/ApiTicketController.php so the MCP update_status tool can
run the exact same code path as the web UI; update_ticket.php now just
require_once's it. The class body is byte-identical to the original
(diffed against HEAD, modulo the 4-space dedent).

Verified the web endpoint over real HTTP with a real session + CSRF:
Open -> In Progress succeeds, In Progress -> Closed without a comment is
refused with requires_comment (400), and with a comment closes the
ticket and persists the reason.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X
2026-09-24 19:13:31 -04:00
jaredandClaude Opus 5.5 65deedc295 Add MCP identity mapping and read tools (#111, phase 4)
Lint / PHP (phpcs PSR-12) (push) Successful in 32s
Lint / JS (eslint) (push) Successful in 10s
Lint / PHP requirements (version + extensions) (push) Successful in 22s
Lint / Notify on failure (push) Skipped
Security / PHP Security (semgrep) (push) Successful in 1m4s
Lint / Deploy (push) Successful in 2s
MCP requests now run as the signed-in Tinker Tickets user, under exactly
the same rules as the web login, and expose the first two tools:
search_tickets and get_ticket.

Identity:
- IdentityMiddleware maps the validated token to a user with the same
  checks as AuthMiddleware: the admin/employee group rule, now extracted
  into helpers/AccessPolicy.php so both entry points share one copy, then
  UserModel::syncUserFromAuthelia(), which creates/updates the row and
  derives is_admin from groups.
- Claims are read from the validated token's server-side PSR-7 request
  attributes, not from JSON-RPC _meta. The SDK's OAuthRequestMetaMiddleware
  is deliberately not used: it array_merges into client-writable _meta,
  so only the keys the validator happens to set are overwritten and a
  client could inject others.

Scopes (ToolScopeMiddleware), enforced before dispatch:
- lifecycle messages need only a valid token; write tools (listed in
  ToolCatalog, the single registry) need tickets:write; everything else
  needs tickets:read, which tickets:write implies.
- Denials are the spec's step-up challenge: 403 +
  WWW-Authenticate: Bearer error="insufficient_scope", scope=...,
  resource_metadata=...

Tools (read-only, annotated readOnlyHint):
- search_tickets: text/status/priority/category/assignee ("me",
  "unassigned", or a username), paginated, via TicketModel::getAllTickets
  with the user's visibility filter. Defaults to every non-Closed status.
- get_ticket: details + comments, gated by canUserAccessTicket. A missing
  ticket and a non-visible one return the same "not found".

Verified locally against a real MariaDB fixture (public, confidential,
internal+group, and closed tickets across two users), driving the real
pipeline (ToolCatalog, both middlewares, SDK transport) with only JWT
validation stubbed: 20/20 checks pass, including visibility parity per
user, confidential tickets hidden from non-owners, the group check
rejecting a user without admin/employee, a missing preferred_username
rejected, 403 insufficient_scope for a token without tickets:*, and
write implying read. Also exercised the stateless 2026-07-28 era (no
session, _meta + MCP-Protocol-Version/Mcp-Method/Mcp-Name headers), which
returns the same visibility-filtered results.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X
2026-09-24 18:58:30 -04:00
jaredandClaude Opus 5.5 13660b4398 Fix duplicated Host header rejecting all MCP requests under PHP-FPM (#111)
Lint / PHP (phpcs PSR-12) (push) Successful in 40s
Lint / JS (eslint) (push) Successful in 12s
Lint / PHP requirements (version + extensions) (push) Successful in 21s
Lint / Notify on failure (push) Skipped
Security / PHP Security (semgrep) (push) Successful in 1m8s
Lint / Deploy (push) Successful in 2s
nyholm's ServerRequestCreator adds Host both from the URI it builds and
from the request headers, so under PHP-FPM getHeaderLine('Host') returns
"beta.t.lotusguild.org, beta.t.lotusguild.org". The SDK's DNS-rebinding
check compares that joined string against the allowlist and refused every
request with 403 "Invalid Host header", even for the correct hostname.
It only passed locally because PHP's built-in server exposes headers
differently.

Collapse Host to the single value the client sent before the middleware
runs. The rebinding check still sees the client's real Host, so foreign
hosts and direct-by-IP access stay refused.

Verified on the beta host by running the patched entrypoint against
beta's real config and vendor/: the correct host gets the Protected
Resource Metadata, and a foreign host still gets 403.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X
2026-09-24 18:46:52 -04:00
jaredandClaude Opus 5.5 5631731a28 Add OAuth-protected MCP endpoint scaffolding (#111, phase 2)
Lint / PHP (phpcs PSR-12) (push) Successful in 51s
Lint / JS (eslint) (push) Successful in 10s
Lint / PHP requirements (version + extensions) (push) Successful in 20s
Lint / Notify on failure (push) Skipped
Security / PHP Security (semgrep) (push) Successful in 2m17s
Lint / Deploy (push) Successful in 7s
First step of the remote MCP server: mcp/server.php serves /mcp over
Streamable HTTP via the official MCP PHP SDK (mcp/sdk, pinned to exactly
0.8.1 since it breaks BC in nearly every minor release), with Authelia as
the OAuth authorization server. No tools yet: this phase only stands up
authentication, RFC 9728 Protected Resource Metadata, and routing.

- Composer is introduced for the MCP endpoint ONLY: nothing else loads
  vendor/autoload.php, so a failed composer install at deploy time can
  only take /mcp down. vendor/ is gitignored and excluded from phpcs;
  composer.lock is resolved for PHP 8.2 so it installs on 8.2 and 8.4.
- Tokens are validated against Authelia's JWKS (cached) for signature,
  issuer, audience == MCP_RESOURCE_URL (a beta token is rejected by prod
  and vice versa), and expiry. scopeClaim is 'scp' because Authelia puts
  scopes in an array claim of that name, not the standard 'scope'.
- The request URI's scheme/host are pinned to MCP_RESOURCE_URL before the
  SDK sees it: TLS ends at the proxy, so PHP sees http and a
  client-controlled Host, and the SDK builds the 401 challenge's
  resource_metadata URL from that. preserveHost keeps the real Host
  header for the DNS-rebinding check, which only allows the canonical
  hostname (so direct-by-IP access is refused too).
- Identity will come only from the token; this entrypoint never reads
  Remote-* headers or $_SESSION, since /mcp is exempt from forward-auth
  at the proxy and those headers are client-controlled there.

Verified locally with PHP's built-in server: unauthenticated POST gets
401 + WWW-Authenticate with the https resource_metadata URL and scopes;
metadata served at both /.well-known/oauth-protected-resource/mcp and the
root form; malformed token -> 401 invalid_token; foreign Host and
direct-IP Host -> 403; a forged Remote-User header without a token is
still 401.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MGDKHiU5RJdo3dqQUDow3X
2026-09-24 18:40:11 -04:00
jaredandClaude Sonnet 5 d0a763079e Merge development into main: API key visibility, notification retry, watch audit logging (#70, #78, #93)
Lint / PHP (phpcs PSR-12) (push) Successful in 35s
Lint / JS (eslint) (push) Successful in 26s
Lint / PHP requirements (version + extensions) (push) Successful in 43s
Lint / Notify on failure (push) Skipped
Security / PHP Security (semgrep) (push) Successful in 1m45s
Lint / Deploy (push) Successful in 2s
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0117oBw2jN4kALYeS8HPq4zV
deploy-2026.09.12-242
2026-09-12 01:20:57 -04:00
jaredandClaude Sonnet 5 35192aaadc Retry failed Matrix webhook notifications with backoff (#78)
Lint / PHP (phpcs PSR-12) (push) Successful in 26s
Lint / JS (eslint) (push) Successful in 7s
Lint / PHP requirements (version + extensions) (push) Successful in 21s
Lint / Notify on failure (push) Skipped
Security / PHP Security (semgrep) (push) Successful in 1m8s
Lint / Deploy (push) Successful in 5s
NotificationHelper::fire() logged a failed webhook post via error_log()
only, with no retry and no persistent record — once a notification
failed, it was gone with no trace beyond the log line, even though the
underlying DB write (audit_log entry, status change, etc.) it was
reporting on had already committed.

Extracted the curl POST into attemptDelivery(), shared between fire()
(unchanged best-effort caller-facing behavior) and the new
cron/retry_failed_notifications.php. On failure, fire() now also queues
the payload to notification_retry_queue (migration 007) via
Database::getConnection() — most fire() call sites don't have a $conn
handy, and threading one through every caller would be a much larger,
more invasive change than reusing the existing connection singleton.

The cron script processes due rows with exponential backoff (2, 4, 8...
capped at 60 minutes) up to each row's max_attempts (default 6), then
leaves an exhausted row in place — not deleted — so it stays visible
for manual investigation instead of disappearing a second time.

Verified against real MariaDB and a local HTTP server standing in for
the Matrix webhook, toggled between failing and succeeding: confirmed
a real failure via fire() is correctly queued; the retry script
reschedules a still-failing row with the expected backoff delay;
flipping the fake webhook to succeed lets the same row's next retry
delete it; a row that exhausts all attempts is left in place and
correctly excluded from the next run's due-row query; and a success
via fire() queues nothing (no regression on the common case).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0117oBw2jN4kALYeS8HPq4zV
2026-09-12 01:18:13 -04:00
jaredandClaude Sonnet 5 6609320c83 Restrict API keys to public-visibility tickets by default (#70)
api/tickets_api.php (both the single-ticket read and the list/triage
path) bypassed ticket visibility entirely for any 'read'-scope key,
regardless of who it was issued to or what it was for — any key got
blanket read access to Confidential and Internal ticket titles,
descriptions, and comments, with no way to scope a key more narrowly.

Added see_all_visibility to api_keys (migration 006), defaulting to
false for both new and existing keys — the prior blanket-access
behavior is what's being restricted here, so unlike scope's own
un-migrated-database fallback (which defaults toward preserving old
behavior), a missing/null value here defaults to the new, restrictive
one. An admin can opt a specific key in via a new checkbox in the API
Key Management UI when it genuinely needs the full queue.

tickets_api.php now builds a synthetic "no special access" user and
runs it through TicketModel's existing per-user visibility plumbing
(getVisibilityFilter/canUserAccessTicket) instead of a separate SQL
path, so this stays in lockstep with however visibility rules evolve
for real users. That synthetic user_id is -1, not 0: testing surfaced
that canUserAccessTicket()'s confidential-ticket check does a PHP-level
(int) cast, and (int)null === 0, so an unassigned confidential ticket's
NULL assigned_to would otherwise false-positive-match a user_id of 0.

Verified against real MariaDB with public/confidential/internal test
tickets: a public-only-scoped key's list only returns the public
ticket, and canUserAccessTicket() correctly returns false for both the
confidential ticket (unassigned, then reassigned to a real user — both
cases) and the internal one; a see_all_visibility key sees all three,
unchanged from the prior behavior. Also verified createKey()/
validateKey()'s default-false and explicit-true paths round-trip
correctly through the real DB.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0117oBw2jN4kALYeS8HPq4zV
2026-09-12 01:17:53 -04:00
jaredandClaude Sonnet 5 e91f4547b6 Log watch/unwatch actions to the audit trail (#93)
api/watch_ticket.php performed the ticket_watchers INSERT IGNORE/DELETE
directly with no AuditLogModel call, unlike every other ticket-adjacent
mutation (comments, attachments, dependencies, status/field changes),
so watching/unwatching never showed up in a ticket's timeline.

Added AuditLogModel::log() calls to both the watch and unwatch paths,
gated on the DB statement's affected_rows so a no-op (already watching,
already not watching) doesn't produce a duplicate timeline entry. Added
'watch'/'unwatch' to AuditLogModel's VALID_ACTION_TYPES, and timeline
rendering in views/TicketView.php ("started watching this ticket" /
"stopped watching this ticket").

Verified against real MariaDB: watch -> unwatch -> watch again produces
exactly 2 timeline entries (not 4) since the two no-op repeats correctly
produced zero rows changed and were not logged; confirmed formatAction()/
getEventIcon() render both action types correctly.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0117oBw2jN4kALYeS8HPq4zV
2026-09-12 01:17:13 -04:00
jaredandClaude Sonnet 5 74544ac5b4 Merge development into main: LDAP avatar lookups now use LDAPS (#95)
Lint / PHP (phpcs PSR-12) (push) Successful in 30s
Lint / JS (eslint) (push) Successful in 16s
Lint / PHP requirements (version + extensions) (push) Successful in 39s
Lint / Notify on failure (push) Skipped
Security / PHP Security (semgrep) (push) Successful in 1m21s
Lint / Deploy (push) Successful in 2s
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0117oBw2jN4kALYeS8HPq4zV
deploy-2026.09.12-238
2026-09-12 00:59:21 -04:00
jaredandClaude Sonnet 5 863f84f37e Switch LDAP avatar lookups to LDAPS (#95)
Lint / PHP (phpcs PSR-12) (push) Successful in 18s
Lint / JS (eslint) (push) Successful in 7s
Lint / PHP requirements (version + extensions) (push) Successful in 28s
Lint / Notify on failure (push) Skipped
Security / PHP Security (semgrep) (push) Successful in 1m56s
Lint / Deploy (push) Successful in 3s
api/user_avatar.php connected via ldap://$ldapHost:$ldapPort — never
ldaps://, and there was no ldap_start_tls() call anywhere in the
codebase. LDAP_BIND_PW was sent over the wire unencrypted on every
avatar fetch.

Switched to ldaps://, and changed LDAP_HOST/LDAP_PORT's defaults to
ldap.lotusguild.org:6360 (lldap's LDAPS listener) instead of the bare
IP on port 3890 (plaintext). PHP's ldap extension verifies the server
cert's hostname by default, so a bare IP won't validate against the
LDAPS cert (issued for *.lotusguild.org) — LDAP_HOST has to be a
hostname the cert covers. This is deliberately not configurable back to
plaintext ldap://.

Infra change (pve-infra, separate repo/commit): added a Pi-hole
split-horizon override so ldap.lotusguild.org resolves internally to
the real LDAP server's LAN IP — its existing public DNS record points
elsewhere (an unrelated host), and there was no internal-only DNS entry
for it before this.

Verified against the real lldap server (pct 147, LDAPS on 6360, a live
Let's Encrypt *.lotusguild.org cert): confirmed the Pi-hole override
resolves correctly from hosts using it as their resolver, then ran the
exact ldap_connect/ldap_bind sequence via `php -r` directly on the
production tinker_tickets host (10.10.10.45) with a deliberately wrong
bind password — got "Invalid credentials" (a real LDAP protocol
response), not a transport/TLS error, proving the full connect + TLS
handshake + hostname verification + bind path works end-to-end in the
actual deployment environment.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0117oBw2jN4kALYeS8HPq4zV
2026-09-12 00:56:00 -04:00
jaredandClaude Sonnet 5 b73a4c792c Merge development into main: dashboard status chart excludes Closed (#110)
Lint / PHP (phpcs PSR-12) (push) Successful in 54s
Lint / JS (eslint) (push) Successful in 9s
Lint / PHP requirements (version + extensions) (push) Successful in 24s
Lint / Notify on failure (push) Skipped
Security / PHP Security (semgrep) (push) Successful in 1m2s
Lint / Deploy (push) Successful in 2s
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0117oBw2jN4kALYeS8HPq4zV
deploy-2026.09.12-234
2026-09-11 22:26:37 -04:00
jaredandClaude Sonnet 5 78ee5fdf48 Exclude Closed tickets from the dashboard status breakdown chart (#110)
Lint / PHP (phpcs PSR-12) (push) Successful in 31s
Lint / JS (eslint) (push) Successful in 13s
Lint / PHP requirements (version + extensions) (push) Successful in 31s
Lint / Notify on failure (push) Skipped
Security / PHP Security (semgrep) (push) Successful in 1m4s
Lint / Deploy (push) Successful in 6s
StatsModel::getAllStats()'s by_status breakdown grouped every status
including Closed, unlike by_priority and by_category which already
filter status != 'Closed'. Closed tickets accumulate indefinitely, so
over time the status donut chart's Closed slice comes to dominate it,
squeezing Open/Pending/In Progress down to barely-visible slivers —
exactly the breakdown the chart exists to show at a glance.

Filtered by_status the same way the other two breakdowns already are.
The separate open_tickets/closed_tickets KPI counts are unaffected (a
different query); Closed just no longer appears as a chart segment.

Note: this issue was labeled 'invalid' with no explanation in the body
or comments — checked and it's Gitea's generic stock label (description
"Something is wrong"), not a documented decision that the request itself
was wrong. The described problem is real and reproducible in the code,
so implementing it as filed.

Verified against real MariaDB: seeded 3 open-ish tickets (Open, Pending,
In Progress) and 3 Closed. by_status now returns only the 3 active
statuses; open_tickets/closed_tickets KPI counts are unchanged at 3/3.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0117oBw2jN4kALYeS8HPq4zV
2026-09-11 22:23:33 -04:00
jaredandClaude Sonnet 5 786674abf3 Merge development into main: user-activity fix + attachment thumbnails (#49, #98)
Lint / PHP (phpcs PSR-12) (push) Successful in 18s
Lint / JS (eslint) (push) Successful in 7s
Lint / PHP requirements (version + extensions) (push) Successful in 20s
Lint / Notify on failure (push) Skipped
Security / PHP Security (semgrep) (push) Successful in 1m48s
Lint / Deploy (push) Successful in 2s
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0117oBw2jN4kALYeS8HPq4zV
deploy-2026.09.12-230
2026-09-11 22:19:57 -04:00
jaredandClaude Sonnet 5 dcf9b0cfa1 Generate real resized thumbnails for image attachments (#98)
Lint / PHP (phpcs PSR-12) (push) Successful in 17s
Lint / JS (eslint) (push) Successful in 7s
Lint / PHP requirements (version + extensions) (push) Successful in 20s
Lint / Notify on failure (push) Skipped
Security / PHP Security (semgrep) (push) Successful in 1m8s
Lint / Deploy (push) Successful in 2s
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
2026-09-11 22:17:35 -04:00
jaredandClaude Sonnet 5 80169de16d Fix User Activity report's Tickets Assigned column to use assignment date (#49)
The "Tickets Assigned" column filtered by tickets.created_at, so a ticket
created outside the selected date range but assigned to a user within it
never counted, while one created in-range but assigned/reassigned later
counted as if the assignment happened in-range — filtered by the wrong
date field for what the column claims to measure.

tickets has no assigned_at column, so derive the count from audit_log's
'assign' events (already logged by both the single-ticket and bulk-assign
paths) instead, filtered by the event's own created_at. COUNT(DISTINCT
entity_id) so a ticket reassigned more than once to the same user within
the range still counts once.

Verified against real MariaDB: seeded one ticket created outside the test
range but assigned inside it, and one created inside the range but
assigned outside it. The old query counted the wrong one; the new query
correctly flips to count the one actually assigned within the range.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0117oBw2jN4kALYeS8HPq4zV
2026-09-11 22:17:25 -04:00
jaredandClaude Sonnet 5 4aa83ffe58 Merge development into main: bulk-op atomicity docs + double-submit guard (#33, #36)
Lint / PHP (phpcs PSR-12) (push) Successful in 18s
Lint / JS (eslint) (push) Successful in 8s
Lint / PHP requirements (version + extensions) (push) Successful in 20s
Lint / Notify on failure (push) Skipped
Security / PHP Security (semgrep) (push) Successful in 1m5s
Lint / Deploy (push) Successful in 2s
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0117oBw2jN4kALYeS8HPq4zV
deploy-2026.09.12-226
2026-09-11 21:58:45 -04:00
jaredandClaude Sonnet 5 844677bbce Fix atomicity docblock and surface per-ticket bulk-op errors (#33)
Lint / PHP (phpcs PSR-12) (push) Successful in 19s
Lint / JS (eslint) (push) Successful in 7s
Lint / PHP requirements (version + extensions) (push) Successful in 23s
Lint / Notify on failure (push) Skipped
Security / PHP Security (semgrep) (push) Successful in 1m4s
Lint / Deploy (push) Successful in 2s
processBulkOperation()'s docblock claimed the transaction "ensures
atomicity - either all tickets are updated or none are," but that's
only true when $atomic = true is passed, and the only real caller
(api/bulk_operation.php) never passes it — the actual default is
best-effort: per-ticket failures are skipped and recorded, and every
other ticket in the batch still commits. Reworded the docblock to
describe the actual default behavior and when $atomic changes it.

The model already collected per-ticket failure reasons into
$result['errors'] (dashboard.js's bulkResultMessage() already reads
data.errors to render them), but api/bulk_operation.php's success
response dropped that field entirely, so admins only ever saw a bare
"N succeeded, M failed" count with no way to see which tickets failed
or why. Added 'errors' to the response when present.

Verified against real MariaDB: a bulk_status operation against a Closed
ticket (no transition defined) and an Open ticket (Open->Pending
defined) correctly processed 1/1, and the API response now includes
errors: ["Ticket ...: transition not allowed (Closed -> Pending)"].

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0117oBw2jN4kALYeS8HPq4zV
2026-09-11 21:56:26 -04:00
jaredandClaude Sonnet 5 3fcd1cbf0e Guard bulk-action buttons against double-submit (#36)
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
2026-09-11 21:56:19 -04:00
jaredandClaude Sonnet 5 3ab33d8df2 Merge development into main: concurrency/atomicity fixes (#34, #35, #37)
Lint / PHP (phpcs PSR-12) (push) Successful in 45s
Lint / JS (eslint) (push) Successful in 13s
Lint / PHP requirements (version + extensions) (push) Successful in 27s
Lint / Notify on failure (push) Skipped
Security / PHP Security (semgrep) (push) Successful in 1m7s
Lint / Deploy (push) Successful in 3s
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0117oBw2jN4kALYeS8HPq4zV
deploy-2026.09.12-222
2026-09-11 21:45:25 -04:00
jaredandClaude Sonnet 5 3778a599c3 Serialize dedup-hash lookups in create_ticket_api.php (#35)
Lint / PHP (phpcs PSR-12) (push) Successful in 21s
Lint / JS (eslint) (push) Successful in 12s
Lint / PHP requirements (version + extensions) (push) Successful in 40s
Lint / Notify on failure (push) Skipped
Security / PHP Security (semgrep) (push) Successful in 1m41s
Lint / Deploy (push) Successful in 2s
Two concurrent hwmonDaemon reports carrying the same dedup hash could
both read the same pre-update ticket snapshot and each independently
apply a priority escalation (losing one), or both attempt to INSERT a
new ticket for a hash that didn't exist yet and have the loser's request
dropped with a "Duplicate ticket" error instead of falling through to
the update/escalate path.

Wrap the hash lookup through the update-or-insert in one transaction,
with the lookup taking SELECT ... FOR UPDATE. For an existing row this
serializes the read-modify-write so a second request observes the
first's committed state. For a not-yet-existing hash, InnoDB's gap lock
there is shared rather than exclusive, so two concurrent inserts can
both reach the INSERT and deadlock (1213) instead of one blocking
cleanly on the other's row; retry the whole lookup once on that
deadlock (or a lock-wait-timeout, 1205) so the retry's own SELECT finds
the winner's committed row and takes the update path instead of erroring.

Verified against real MariaDB with two concurrent OS processes for both
scenarios: (1) same existing active ticket — the second process blocked
~1.1s on the first's held row lock, then correctly escalated from the
first's committed priority rather than a stale value; (2) same
not-yet-existing hash — reproduced the 1213 deadlock deterministically
across 5/5 runs with the original code, then confirmed the retry
resolves it every time (5/5), leaving exactly one ticket row created and
no dropped/erroring request.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0117oBw2jN4kALYeS8HPq4zV
2026-09-11 21:41:57 -04:00
jaredandClaude Sonnet 5 310dcd0840 Persist status-change comments transactionally with the update (#37)
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
2026-09-11 21:41:48 -04:00
jaredandClaude Sonnet 5 d205a9577a Fix TOCTOU race in bulk operations by row-locking tickets (#34)
processBulkOperation() validated each ticket's status/priority transition
against a pre-transaction snapshot (ticketsById) instead of re-reading
inside the transaction, so two concurrent bulk operations touching the
same ticket could both pass validation against stale data and one
transition could silently clobber the other. Add lockTicketForUpdate(),
which re-fetches a ticket via SELECT ... FOR UPDATE, and use it wherever
the loop needs current status/priority, removing the three redundant
re-reads from the stale snapshot in the bulk_close/bulk_priority/
bulk_status branches.

Verified against real MariaDB with two concurrent OS processes: the
second process blocked ~1.2s on the first's held row lock, then correctly
observed the first's committed status and rejected an otherwise-stale-
data-permitted invalid transition. Also regression-tested normal
bulk_close/bulk_priority operation on real tickets.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0117oBw2jN4kALYeS8HPq4zV
2026-09-11 21:41:41 -04:00
jared 5572f0be45 Merge development into main: quick/contained cleanup batch (#43, #44, #45, #109)
Lint / PHP (phpcs PSR-12) (push) Successful in 26s
Lint / JS (eslint) (push) Successful in 13s
Lint / PHP requirements (version + extensions) (push) Successful in 31s
Lint / Notify on failure (push) Skipped
Security / PHP Security (semgrep) (push) Successful in 1m34s
Lint / Deploy (push) Successful in 2s
- Document intentional empty catches, fix one real gap, allow == null in eqeqeq (#44)
- Remove unused lt.markdown module (#43)
- Update stale README Project Structure tree and migrations docs (#45)
- Add a table-insert toolbar button to the markdown editor (#109)
deploy-2026.09.11-218
2026-09-11 15:20:11 -04:00
jaredandClaude Sonnet 5 1600412a6d Add a table-insert toolbar button to the markdown editor (#109)
Lint / PHP (phpcs PSR-12) (push) Successful in 31s
Lint / JS (eslint) (push) Successful in 10s
Lint / PHP requirements (version + extensions) (push) Successful in 29s
Lint / Notify on failure (push) Skipped
Security / PHP Security (semgrep) (push) Successful in 3m1s
Lint / Deploy (push) Successful in 5s
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
2026-09-11 15:11:03 -04:00
jaredandClaude Sonnet 5 803c65616b Update stale README Project Structure tree and migrations docs (#45)
Cross-checking README.md against the actual file tree found three gaps:
views/error_403.php and error_404.php (plus error_500.php, added since
the issue was filed) weren't listed under views/; config/requirements.php
wasn't listed under config/ (confirmed it's not a duplicate of
scripts/check_requirements.php — it's the shared data source both that
script and api/health.php read from); and the Database Schema section
only described 000_baseline.sql and migrate.php generically, with no
mention that four numbered migrations now exist on top of the baseline.

Updated the Project Structure tree and the Database Schema/Migrations
prose to list all of these, noting that 000_baseline.sql already
includes every numbered migration's changes for a fresh install (they
only matter when upgrading an existing database).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Lhz7pGMaoTfL5sdYS5XiKv
2026-09-11 15:10:56 -04:00
jaredandClaude Sonnet 5 0e163f6607 Remove unused lt.markdown module (#43)
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
2026-09-11 15:10:47 -04:00
jaredandClaude Sonnet 5 e1448d8ea2 Document intentional empty catches, fix one real gap, allow == null in eqeqeq (#44)
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
2026-09-11 15:08:50 -04:00
jared b6c17096b5 Merge development into main: search filter merge, workflow dup rejection, recurring-ticket loss alert, preview debounce (#58, #62, #88, #108)
Lint / PHP (phpcs PSR-12) (push) Successful in 32s
Lint / JS (eslint) (push) Successful in 10s
Lint / PHP requirements (version + extensions) (push) Successful in 27s
Lint / Notify on failure (push) Skipped
Security / PHP Security (semgrep) (push) Successful in 1m33s
Lint / Deploy (push) Successful in 2s
- Merge into current URL params instead of replacing them in Advanced Search (#58)
- Reject duplicate workflow transitions with a clear error (#62)
- Alert and record a lost recurring-ticket occurrence on creation failure (#88)
- Debounce the live markdown preview (#108)
deploy-2026.09.11-214
2026-09-11 14:49:44 -04:00
jaredandClaude Sonnet 5 6bd1bb082a Debounce the live markdown preview (#108)
Lint / PHP (phpcs PSR-12) (push) Successful in 28s
Lint / JS (eslint) (push) Successful in 10s
Lint / PHP requirements (version + extensions) (push) Successful in 31s
Lint / Notify on failure (push) Skipped
Security / PHP Security (semgrep) (push) Successful in 1m31s
Lint / Deploy (push) Successful in 2s
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
2026-09-11 14:28:39 -04:00
jaredandClaude Sonnet 5 a4828c1b7b Alert and record a lost recurring-ticket occurrence on creation failure (#88)
RecurringTicketModel::claimForRun() deliberately advances next_run_at
before TicketModel::createTicket() runs, to prevent duplicate-ticket
floods if creation fails partway and the cron retries. The tradeoff:
if createTicket() then fails, that specific occurrence is gone forever
with no record anywhere an admin would normally look — the catch
block only wrote a line to stdout/the cron log.

Added recordMissedOccurrence(), called from both the "createTicket()
returned success:false" branch and the exception catch, which writes
an audit_log entry (entity_type='recurring_ticket', action_type='error')
and fires a new NotificationHelper::sendSystemAlert() — a generic
operational alert (unlike the ticket-specific notification methods,
it has no associated ticket) sent to the shared MATRIX_NOTIFY_USERS
list regardless of any per-event toggle, so a silently-skipped
recurring ticket surfaces immediately instead of requiring someone to
grep cron logs.

Verified against real MariaDB and a real webhook-capturing server:
calling the recorder writes the audit_log row with the failure reason
and schedule details, and fires the Matrix alert with the same
information.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Lhz7pGMaoTfL5sdYS5XiKv
2026-09-11 14:28:31 -04:00
jaredandClaude Sonnet 5 86ef91abcb Reject duplicate workflow transitions with a clear error (#62)
status_transitions already has a DB-level UNIQUE KEY on
(from_status, to_status), so a genuine duplicate pair was never
actually possible to insert — but hitting that constraint raw
surfaced as an opaque "An internal error occurred" to the admin
instead of a clear message, since manage_workflows.php only validated
from_status !== to_status before attempting the insert/update.

Added an explicit existence check before insert/update in both the
POST and PUT handlers (excluding the row's own ID on update), so the
common case — an admin re-adding or renaming into a pair that already
exists — gets a specific 409 with the conflicting pair named, instead
of a generic 500. Also added ORDER BY transition_id to
WorkflowModel::getAllTransitions() as a defense-in-depth backstop:
since it collapses rows into a PHP array keyed by
[from_status][to_status] with no defined winner otherwise, if the DB
constraint were ever weakened or bypassed, this at least makes which
row wins deterministic (most recently created).

Verified against a real running server + real MariaDB: creating a
duplicate active pair, a duplicate inactive pair, and updating a
different row into an existing pair are all correctly rejected with
the friendly message; updating a row to keep its own existing pair
succeeds; and a genuinely different pair still creates normally.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Lhz7pGMaoTfL5sdYS5XiKv
2026-09-11 14:28:21 -04:00
jaredandClaude Sonnet 5 6d68af40e7 Merge into current URL params instead of replacing them in Advanced Search (#58)
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
2026-09-11 14:28:12 -04:00
jared aa8173941a Merge development into main: webhook timeout, comment-edit timeline fix, Bearer rate-limiting overhaul (#77, #80, #81, #82, #83, #87)
Lint / PHP (phpcs PSR-12) (push) Successful in 43s
Lint / JS (eslint) (push) Successful in 12s
Lint / PHP requirements (version + extensions) (push) Successful in 30s
Lint / Notify on failure (push) Skipped
Security / PHP Security (semgrep) (push) Successful in 2m4s
Lint / Deploy (push) Successful in 3s
- Add connect-timeout to Matrix webhook calls (#77)
- Include ticket_id in comment-edit audit log so it appears on the timeline (#87)
- Overhaul Bearer API rate limiting: real config, per-key isolation, skip session (#80, #81, #82, #83)
deploy-2026.09.11-210
2026-09-11 14:11:48 -04:00
jaredandClaude Sonnet 5 1b1801696f Overhaul Bearer API rate limiting: real config, per-key isolation, skip session (#80, #81, #82, #83)
Lint / PHP (phpcs PSR-12) (push) Successful in 28s
Lint / JS (eslint) (push) Successful in 12s
Lint / PHP requirements (version + extensions) (push) Successful in 30s
Lint / Notify on failure (push) Skipped
Security / PHP Security (semgrep) (push) Successful in 3m9s
Lint / Deploy (push) Successful in 2s
Four interrelated gaps in the same rate-limiting path:

- #80: RATE_LIMIT_DEFAULT/RATE_LIMIT_API were defined in config.php but
  RateLimitMiddleware never read them (hardcoded class constants
  instead), and they weren't in .env.example — a deployer editing them
  saw zero effect with no documented way to actually change the limit.
- #81: Bearer traffic was rate-limited purely by a shared IP bucket
  (the session-based half was a no-op for stateless clients, since a
  fresh session starts on every request). Two different API keys from
  the same host/NAT egress IP shared ONE bucket, so a chatty or
  misbehaving key could 429 a completely unrelated key's traffic.
- #82: X-RateLimit-* headers reported the meaningless session counter
  for Bearer clients instead of whatever bucket actually governed them.
- #83: RateLimitMiddleware::check() called session_start()
  unconditionally, before ApiKeyAuth even runs — continuous session-file
  churn and an unnecessary Set-Cookie on every stateless API request,
  using un-hardened cookie defaults since it runs before
  AuthMiddleware's hardening (which Bearer requests never reach anyway).

Fixed as one pass since they're the same code path: config.php now
reads RATE_LIMIT_DEFAULT/RATE_LIMIT_API from .env (added there too,
documented); the middleware now extracts the raw Bearer token
(independent of ApiKeyAuth, so no DB round-trip needed before rate
limiting, and it works whether or not the token later turns out
valid) and rate-limits it via its own per-token bucket instead of
starting a session — the existing IP-based bucket still applies
underneath as defense-in-depth against volumetric abuse from one
network path, but each distinct key now gets real isolated headroom.
getStatus()/addHeaders() report that per-token bucket for Bearer
requests instead of the session counter.

Verified: a Bearer request creates zero session files (confirmed via
real session-directory file count before/after); two different keys
from different IPs are fully isolated (one exhausting its own 120/min
bucket has zero effect on the other); a config-driven RATE_LIMIT_API
override (e.g. 5) is correctly honored for session-based (non-Bearer)
traffic; X-RateLimit-* status correctly reflects the per-key bucket
for a Bearer request.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Lhz7pGMaoTfL5sdYS5XiKv
2026-09-11 14:05:14 -04:00
jaredandClaude Sonnet 5 09cea2b388 Include ticket_id in comment-edit audit log so it appears on the timeline (#87)
AuditLogModel::getTicketTimeline() requires, for entity_type='comment'
rows, that details.ticket_id match the ticket being viewed.
logCommentCreate() and delete-comment's audit call both correctly
include it; update_comment.php's audit call only set
comment_text_preview, so an edited comment's audit row was written
(visible in the admin's global Audit Log) but never matched the
timeline's join condition — a comment edit left no trace on the
ticket's own history, while deleting the same comment would be
visible.

Added ticket_id to the details array, using $comment['ticket_id']
already loaded earlier in the file for the access check. Verified
against real MariaDB: the fixed shape now correctly appears in
getTicketTimeline()'s results.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Lhz7pGMaoTfL5sdYS5XiKv
2026-09-11 14:05:01 -04:00
jaredandClaude Sonnet 5 9702aafacd Add connect-timeout to Matrix webhook calls (#77)
NotificationHelper::fire() set CURLOPT_TIMEOUT (10s total) but no
CURLOPT_CONNECTTIMEOUT, so a slow-but-not-hung hookshot endpoint could
add up to the full 10s per fire() call — and a single request can call
fire() more than once sequentially (e.g. add_comment.php firing
mention + comment + watcher notifications back to back), stacking into
tens of seconds of added latency on the user-facing response.

Added a 3s CURLOPT_CONNECTTIMEOUT so a slow-to-connect endpoint fails
fast without needing the full request to time out. Verified the
webhook still fires correctly end-to-end against a real local HTTP
server after the change.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Lhz7pGMaoTfL5sdYS5XiKv
2026-09-11 14:04:52 -04:00
jared 66bf82bf46 Merge development into main: visibility-notification pruning + CSRF UX + custom field type validation (#48, #50, #57, #73, #86)
Lint / PHP (phpcs PSR-12) (push) Successful in 30s
Lint / JS (eslint) (push) Successful in 10s
Lint / PHP requirements (version + extensions) (push) Successful in 32s
Lint / Notify on failure (push) Skipped
Security / PHP Security (semgrep) (push) Successful in 3m22s
Lint / Deploy (push) Successful in 6s
- 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)
deploy-2026.09.11-206
2026-09-11 13:48:20 -04:00
jaredandClaude Sonnet 5 fca0b42726 Validate field_type against the allowed enum in custom field definitions (#50)
Lint / PHP (phpcs PSR-12) (push) Successful in 31s
Lint / JS (eslint) (push) Successful in 13s
Lint / PHP requirements (version + extensions) (push) Successful in 39s
Lint / Notify on failure (push) Skipped
Security / PHP Security (semgrep) (push) Successful in 2m1s
Lint / Deploy (push) Successful in 6s
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
2026-09-11 13:31:33 -04:00
jaredandClaude Sonnet 5 ae12fcd6fd Auto-retry once after CSRF token resync in lt.api (#86)
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
2026-09-11 13:31:25 -04:00
jaredandClaude Sonnet 5 5b96e75ff6 Use lt.api instead of raw fetch() in notification bell (#57)
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
2026-09-11 13:31:15 -04:00
jaredandClaude Sonnet 5 3db3749c46 Re-check ticket visibility before surfacing in-app notifications (#48)
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
2026-09-11 13:31:08 -04:00
jaredandClaude Sonnet 5 9e83f8903a Prune watchers when a ticket's visibility is tightened (#73)
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
2026-09-11 13:30:59 -04:00
jared cabdae35cc Merge development into main: Custom Fields wiring (#47)
Lint / PHP (phpcs PSR-12) (push) Successful in 31s
Lint / JS (eslint) (push) Successful in 14s
Lint / PHP requirements (version + extensions) (push) Successful in 53s
Lint / Notify on failure (push) Skipped
Security / PHP Security (semgrep) (push) Successful in 1m43s
Lint / Deploy (push) Successful in 4s
- Wire Custom Fields into ticket creation and viewing (#47)
deploy-2026.09.11-202
2026-09-11 13:17:33 -04:00