Compare commits

...
Author SHA1 Message Date
jaredandClaude Sonnet 5 5709c3134f Verify mentioned-user access before sending @mention notifications (#69)
Lint / PHP (phpcs PSR-12) (push) Successful in 40s
Lint / JS (eslint) (push) Successful in 16s
Lint / PHP requirements (version + extensions) (push) Successful in 47s
Lint / Notify on failure (push) Skipped
Security / PHP Security (semgrep) (push) Successful in 2m31s
Lint / Deploy (push) Successful in 4s
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
2026-09-08 21:49:42 -04:00
jaredandClaude Sonnet 5 60bafae8a0 Redact ticket title for non-public tickets in assignment notifications (#72)
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
2026-09-08 21:49:34 -04:00
jaredandClaude Sonnet 5 90d798966b Exclude shared notify list and redact title in notifyWatchers() for non-public tickets (#71)
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
2026-09-08 21:49:17 -04:00
jaredandClaude Sonnet 5 442cd1d6f6 Redact ticket title for non-public tickets in create/status-change Matrix notifications (#46)
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
2026-09-08 21:49:06 -04:00
jared 3664719148 Merge development into main: high-priority security/reliability batch (#27, #28, #30, #32)
Lint / PHP (phpcs PSR-12) (push) Successful in 29s
Lint / JS (eslint) (push) Successful in 10s
Lint / PHP requirements (version + extensions) (push) Successful in 25s
Lint / Notify on failure (push) Skipped
Security / PHP Security (semgrep) (push) Successful in 1m44s
Lint / Deploy (push) Successful in 6s
- 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)
2026-09-08 21:33:33 -04:00
jaredandClaude Sonnet 5 d7940b1e31 Fix ticket_watchers.ticket_id type mismatch and missing FK (#32)
Lint / PHP (phpcs PSR-12) (push) Successful in 24s
Lint / JS (eslint) (push) Successful in 9s
Lint / PHP requirements (version + extensions) (push) Successful in 27s
Lint / Notify on failure (push) Skipped
Security / PHP Security (semgrep) (push) Successful in 1m40s
Lint / Deploy (push) Successful in 2s
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
2026-09-08 21:28:42 -04:00
jaredandClaude Sonnet 5 fba251b85d Replace illusory transaction wrapping in migrate.php with statement-level resume (#30)
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
2026-09-08 21:28:32 -04:00
jaredandClaude Sonnet 5 fd777aa690 Fix visibility-group matching disagreement between filter and access check (#28)
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
2026-09-08 21:28:25 -04:00
jaredandClaude Sonnet 5 a9a39adcf8 Add missing rate limiting to create_ticket_api.php (#27)
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
2026-09-08 21:28:18 -04:00
13 changed files with 194 additions and 53 deletions
+10 -3
View File
@@ -177,9 +177,16 @@ try {
$ticketTitle = $ticket['title'] ?? "Ticket #{$ticketId}";
$ticketVisibility = $ticket['visibility'] ?? 'public';
// @mention notifications — resolve usernames → Matrix IDs via Synapse Admin API
if (!empty($mentionedUsers)) {
$mentionedUsernames = array_column($mentionedUsers, 'username');
// @mention notifications — resolve usernames → Matrix IDs via Synapse Admin API.
// Only notify mentioned users who actually have access to this ticket;
// otherwise a mention would DM them the ticket's title and comment text
// even though canUserAccessTicket() would deny them the ticket itself.
$accessibleMentionedUsers = array_filter(
$mentionedUsers,
fn($u) => $ticketModel->canUserAccessTicket($ticket, $u)
);
if (!empty($accessibleMentionedUsers)) {
$mentionedUsernames = array_column($accessibleMentionedUsers, 'username');
$mentionedMatrixIds = SynapseHelper::resolveUsernames($mentionedUsernames);
if (!empty($mentionedMatrixIds)) {
NotificationHelper::sendMentionNotification($ticketId, $ticketTitle, $commentText, $authorDisplay, $mentionedMatrixIds);
+2 -1
View File
@@ -76,7 +76,8 @@ if ($assignedTo === null || $assignedTo === '') {
$ticket['title'] ?? "Ticket #{$ticketId}",
$assigneeName,
$assigneeMatrix,
$changedByDisplay
$changedByDisplay,
$ticket['visibility'] ?? 'public'
);
}
}
+1 -1
View File
@@ -138,7 +138,7 @@ $statusSql = "SELECT DISTINCT
COALESCE(u.display_name, u.username, 'System') AS actor_name
FROM audit_log al
LEFT JOIN users u ON al.user_id = u.user_id
INNER JOIN ticket_watchers tw ON tw.ticket_id = CAST(al.entity_id AS UNSIGNED) AND tw.user_id = ?
INNER JOIN ticket_watchers tw ON tw.ticket_id = al.entity_id AND tw.user_id = ?
WHERE al.action_type = 'update'
AND al.entity_type = 'ticket'
AND al.user_id != ?
+2 -1
View File
@@ -171,7 +171,8 @@ if ($currentStatus !== $newStatus) {
$currentStatus,
$newStatus,
(string)$ticket['title'],
$keyName
$keyName,
$ticket['visibility'] ?? 'public'
);
NotificationHelper::notifyWatchers(
$conn,
+2 -1
View File
@@ -267,7 +267,8 @@ try {
$currentTicket['status'],
$updateData['status'],
$updateData['title'],
$changedBy
$changedBy,
$currentTicket['visibility'] ?? 'public'
);
NotificationHelper::notifyWatchers(
$this->conn,
+19 -14
View File
@@ -12,40 +12,43 @@ require_once dirname(__DIR__) . '/models/TicketModel.php';
$data = json_decode(file_get_contents('php://input'), true) ?? [];
$ticketId = isset($_GET['ticket_id'])
? (int)$_GET['ticket_id']
: (int)($data['ticket_id'] ?? 0);
$ticketIdRaw = isset($_GET['ticket_id']) ? $_GET['ticket_id'] : ($data['ticket_id'] ?? '');
if ($_SERVER['REQUEST_METHOD'] === 'POST') {
$ticketId = (int)($data['ticket_id'] ?? 0);
$ticketIdRaw = $data['ticket_id'] ?? '';
$action = $data['action'] ?? '';
if ($ticketId <= 0 || !in_array($action, ['watch', 'unwatch'], true)) {
if ($ticketIdRaw === '' || !in_array($action, ['watch', 'unwatch'], true)) {
http_response_code(400);
echo json_encode(['success' => false, 'error' => 'Invalid parameters']);
exit;
}
$ticketModel = new TicketModel($conn);
$ticket = $ticketModel->getTicketById($ticketId);
$ticket = $ticketModel->getTicketById((string)$ticketIdRaw);
if (!$ticket || !$ticketModel->canUserAccessTicket($ticket, $currentUser)) {
http_response_code(404);
echo json_encode(['success' => false, 'error' => 'Ticket not found']);
exit;
}
// Use the canonical ticket_id string from the fetched ticket row, not the
// raw request value, so ticket_watchers always stores exactly what's in
// tickets.ticket_id.
$ticketId = $ticket['ticket_id'];
if ($action === 'watch') {
$stmt = $conn->prepare(
"INSERT IGNORE INTO ticket_watchers (ticket_id, user_id) VALUES (?, ?)"
);
$stmt->bind_param("ii", $ticketId, $userId);
$stmt->bind_param("si", $ticketId, $userId);
$stmt->execute();
$stmt->close();
} else {
$stmt = $conn->prepare(
"DELETE FROM ticket_watchers WHERE ticket_id = ? AND user_id = ?"
);
$stmt->bind_param("ii", $ticketId, $userId);
$stmt->bind_param("si", $ticketId, $userId);
$stmt->execute();
$stmt->close();
}
@@ -54,7 +57,7 @@ if ($_SERVER['REQUEST_METHOD'] === 'POST') {
$countStmt = $conn->prepare(
"SELECT COUNT(*) as cnt FROM ticket_watchers WHERE ticket_id = ?"
);
$countStmt->bind_param("i", $ticketId);
$countStmt->bind_param("s", $ticketId);
$countStmt->execute();
$count = (int)$countStmt->get_result()->fetch_assoc()['cnt'];
$countStmt->close();
@@ -73,7 +76,7 @@ if ($_SERVER['REQUEST_METHOD'] !== 'GET') {
exit;
}
if ($ticketId <= 0) {
if ($ticketIdRaw === '') {
http_response_code(400);
echo json_encode(['success' => false, 'error' => 'ticket_id required']);
exit;
@@ -83,17 +86,19 @@ if ($ticketId <= 0) {
// restricted ticket's watcher list and count aren't disclosed (the POST path
// already checks this).
$ticketModel = new TicketModel($conn);
$ticket = $ticketModel->getTicketById($ticketId);
$ticket = $ticketModel->getTicketById((string)$ticketIdRaw);
if (!$ticket || !$ticketModel->canUserAccessTicket($ticket, $currentUser)) {
http_response_code(404);
echo json_encode(['success' => false, 'error' => 'Ticket not found']);
exit;
}
$ticketId = $ticket['ticket_id'];
$watchingStmt = $conn->prepare(
"SELECT COUNT(*) as cnt FROM ticket_watchers WHERE ticket_id = ? AND user_id = ?"
);
$watchingStmt->bind_param("ii", $ticketId, $userId);
$watchingStmt->bind_param("si", $ticketId, $userId);
$watchingStmt->execute();
$watching = (bool)$watchingStmt->get_result()->fetch_assoc()['cnt'];
$watchingStmt->close();
@@ -107,7 +112,7 @@ $watchersStmt = $conn->prepare(
ORDER BY tw.created_at ASC
LIMIT 6"
);
$watchersStmt->bind_param("i", $ticketId);
$watchersStmt->bind_param("s", $ticketId);
$watchersStmt->execute();
$watchersResult = $watchersStmt->get_result();
$watchers = [];
@@ -118,7 +123,7 @@ $watchersStmt->close();
// True watcher count (the list above is capped at 6 for the avatar group)
$countStmt = $conn->prepare("SELECT COUNT(*) AS cnt FROM ticket_watchers WHERE ticket_id = ?");
$countStmt->bind_param("i", $ticketId);
$countStmt->bind_param("s", $ticketId);
$countStmt->execute();
$count = (int)$countStmt->get_result()->fetch_assoc()['cnt'];
$countStmt->close();
+3
View File
@@ -5,6 +5,9 @@ header('Content-Type: application/json');
error_reporting(E_ALL);
ini_set('display_errors', 0);
require_once __DIR__ . '/middleware/RateLimitMiddleware.php';
RateLimitMiddleware::apply('api');
// Load environment variables with error check
$envFile = __DIR__ . '/.env';
if (!file_exists($envFile)) {
+50 -16
View File
@@ -40,20 +40,40 @@ class NotificationHelper
return array_values(array_filter(array_map('trim', explode(',', $raw))));
}
/**
* Redact a ticket title for the shared Matrix notify list when the
* ticket isn't public, matching how sendCommentNotification() and
* notifyWatchers() already redact comment/activity previews for the
* same list.
*/
private static function redactedTitle(string $title, string $visibility): string
{
return $visibility === 'public' ? $title : '(restricted ticket — title hidden)';
}
// ─── Public event methods ─────────────────────────────────────────────────
/**
* New ticket created (manual or automated/API).
*
* $ticketData['visibility'] ('public', 'internal', or 'confidential') is
* used to redact the title sent to the shared MATRIX_NOTIFY_USERS list
* for non-public tickets, same as sendCommentNotification()'s preview
* redaction. Defaults to 'public' for callers (e.g. the hwmonDaemon
* Bearer-API paths) that never set a non-default visibility.
*/
public static function sendTicketNotification($ticketId, array $ticketData, string $trigger = 'manual'): void
{
preg_match('/^\[([^\]]+)\]/', $ticketData['title'] ?? '', $m);
$visibility = $ticketData['visibility'] ?? 'public';
$title = $ticketData['title'] ?? 'Untitled';
preg_match('/^\[([^\]]+)\]/', $title, $m);
$source = $m[1] ?? ($trigger === 'automated' ? 'Automated' : 'Manual');
self::fire([
'event' => 'ticket_created',
'ticket_id' => $ticketId,
'title' => $ticketData['title'] ?? 'Untitled',
'title' => self::redactedTitle($title, $visibility),
'priority' => (int)($ticketData['priority'] ?? 4),
'category' => $ticketData['category'] ?? 'General',
'type' => $ticketData['type'] ?? 'Issue',
@@ -73,13 +93,16 @@ class NotificationHelper
* @param string $newStatus
* @param string $ticketTitle
* @param string|null $changedByDisplay Display name of the user who changed status
* @param string $visibility Ticket visibility; non-public titles are
* redacted before being sent to the shared
* notify list, same as sendTicketNotification().
*/
public static function sendStatusChangeNotification($ticketId, string $oldStatus, string $newStatus, string $ticketTitle, ?string $changedByDisplay = null): void
public static function sendStatusChangeNotification($ticketId, string $oldStatus, string $newStatus, string $ticketTitle, ?string $changedByDisplay = null, string $visibility = 'public'): void
{
self::fire([
'event' => 'status_changed',
'ticket_id' => $ticketId,
'title' => $ticketTitle,
'title' => self::redactedTitle($ticketTitle, $visibility),
'old_status' => $oldStatus,
'new_status' => $newStatus,
'changed_by' => $changedByDisplay,
@@ -166,11 +189,12 @@ class NotificationHelper
* @param array $extraData Merged into the payload (old_status/new_status, author, etc.)
* @param int|null $excludeUserId Don't notify the actor themselves
* @param string $visibility Ticket visibility: 'public', 'internal', or
* 'confidential'. notify_users includes the
* shared list, which may contain users without
* access to non-public tickets, so any comment
* body preview in $extraData is redacted for
* non-public tickets.
* 'confidential'. The shared notify list may
* contain users without access to non-public
* tickets, so for those tickets it's excluded
* entirely (only actual watchers are notified)
* and both the title and any comment/body
* preview in $extraData are redacted.
*/
public static function notifyWatchers(\mysqli $conn, $ticketId, string $ticketTitle, string $event, array $extraData = [], ?int $excludeUserId = null, string $visibility = 'public'): void
{
@@ -204,9 +228,9 @@ class NotificationHelper
return;
}
if ($excludeUserId !== null) {
$stmt->bind_param("ii", $ticketId, $excludeUserId);
$stmt->bind_param("si", $ticketId, $excludeUserId);
} else {
$stmt->bind_param("i", $ticketId);
$stmt->bind_param("s", $ticketId);
}
$stmt->execute();
$result = $stmt->get_result();
@@ -230,13 +254,17 @@ class NotificationHelper
return;
}
// Remove the global notify list duplicates and build payload
$allNotify = array_unique(array_merge($matrixIds, self::notifyUsers()));
// The shared notify list may include users without access to
// non-public tickets, so only mix it in for public tickets — for
// internal/confidential tickets, notify actual watchers only.
$allNotify = $visibility === 'public'
? array_unique(array_merge($matrixIds, self::notifyUsers()))
: $matrixIds;
$payload = array_merge($extraData, [
'event' => $event,
'ticket_id' => $ticketId,
'title' => $ticketTitle,
'title' => self::redactedTitle($ticketTitle, $visibility),
'url' => UrlHelper::ticketUrl($ticketId),
'notify_users' => array_values($allNotify),
]);
@@ -252,8 +280,14 @@ class NotificationHelper
* @param string|null $assigneeName Display name of new assignee
* @param string|null $assigneeMatrix Matrix user ID of new assignee (to DM)
* @param string|null $changedByDisplay
* @param string $visibility Ticket visibility; non-public titles are
* redacted before being sent to the shared
* notify list, same as sendTicketNotification().
* The assignee is DMed directly regardless,
* since they now have standing access to the
* ticket by virtue of being assigned to it.
*/
public static function sendAssignmentNotification($ticketId, string $ticketTitle, ?string $assigneeName, ?string $assigneeMatrix, ?string $changedByDisplay = null): void
public static function sendAssignmentNotification($ticketId, string $ticketTitle, ?string $assigneeName, ?string $assigneeMatrix, ?string $changedByDisplay = null, string $visibility = 'public'): void
{
$notifyUsers = self::notifyUsers();
// Also notify the assignee directly if we know their Matrix ID
@@ -267,7 +301,7 @@ class NotificationHelper
self::fire([
'event' => 'assigned',
'ticket_id' => $ticketId,
'title' => $ticketTitle,
'title' => self::redactedTitle($ticketTitle, $visibility),
'assignee' => $assigneeName,
'changed_by' => $changedByDisplay,
'url' => UrlHelper::ticketUrl($ticketId),
+3 -2
View File
@@ -243,11 +243,12 @@ CREATE TABLE IF NOT EXISTS `ticket_templates` (
-- ============ ticket_watchers ============
CREATE TABLE IF NOT EXISTS `ticket_watchers` (
`ticket_id` int(11) NOT NULL,
`ticket_id` varchar(9) NOT NULL,
`user_id` int(11) NOT NULL,
`created_at` timestamp NOT NULL DEFAULT current_timestamp(),
PRIMARY KEY (`ticket_id`,`user_id`),
KEY `idx_watcher_user` (`user_id`)
KEY `idx_watcher_user` (`user_id`),
CONSTRAINT `fk_watchers_ticket_id` FOREIGN KEY (`ticket_id`) REFERENCES `tickets` (`ticket_id`) ON DELETE CASCADE
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_general_ci;
-- ============ tickets ============
@@ -0,0 +1,28 @@
-- Fix ticket_watchers.ticket_id type mismatch and missing FK to tickets
--
-- ticket_watchers.ticket_id was int(11), while every other satellite table
-- (ticket_comments, ticket_attachments, ticket_dependencies,
-- custom_field_values) stores it as varchar(9)/varchar(10) matching
-- tickets.ticket_id. There was also no FK constraint at all, unlike every
-- other satellite table, so orphaned watcher rows could never be caught by
-- referential integrity. Ticket IDs are always 9-digit numeric strings
-- (see TicketModel::create's sprintf('%09d', ...)), so the int -> varchar(9)
-- conversion below is lossless for real data.
--
-- Safe to re-run.
-- Remove any watcher rows that no longer point at a real ticket (possible
-- today precisely because there was no FK to prevent it) before adding the
-- constraint, since orphans would make the ADD CONSTRAINT below fail.
DELETE tw FROM `ticket_watchers` tw
LEFT JOIN `tickets` t ON tw.`ticket_id` = t.`ticket_id`
WHERE t.`ticket_id` IS NULL;
ALTER TABLE `ticket_watchers`
MODIFY COLUMN `ticket_id` varchar(9) NOT NULL;
ALTER TABLE `ticket_watchers`
DROP FOREIGN KEY IF EXISTS `fk_watchers_ticket_id`;
ALTER TABLE `ticket_watchers`
ADD CONSTRAINT `fk_watchers_ticket_id` FOREIGN KEY (`ticket_id`) REFERENCES `tickets` (`ticket_id`) ON DELETE CASCADE;
+68 -11
View File
@@ -46,6 +46,23 @@ if (!$conn->query($createTable)) {
exit(1);
}
// Tracks per-statement progress within a migration file. MySQL DDL statements
// (ALTER/CREATE TABLE, etc.) cause an implicit commit, so begin_transaction()/
// rollback() around a whole file can't actually undo DDL already executed
// earlier in that same file. This table lets a re-run after a partial failure
// resume from the statement after the last one that succeeded, instead of
// re-executing already-applied DDL and wedging on "already exists" errors.
$createProgressTable = "CREATE TABLE IF NOT EXISTS migration_progress (
filename VARCHAR(255) NOT NULL PRIMARY KEY,
last_statement_index INT NOT NULL,
updated_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP
)";
if (!$conn->query($createProgressTable)) {
echo "Error: Could not create migration_progress table: " . $conn->error . "\n";
exit(1);
}
// Get list of completed migrations
$completed = [];
$result = $conn->query("SELECT filename FROM migrations ORDER BY id");
@@ -114,47 +131,87 @@ foreach ($pending as $file) {
continue;
}
// Execute migration - handle multiple statements
$conn->begin_transaction();
// Execute migration statement-by-statement, tracking progress as we go.
// No begin_transaction()/rollback() here: DDL statements auto-commit in
// MySQL/MariaDB regardless, so a transaction wrapper around the whole
// file would only create the illusion of atomicity while giving no real
// protection. Instead, each statement commits immediately (autocommit),
// and its index is durably recorded so a later re-run can resume exactly
// where a previous run left off rather than re-executing already-applied
// DDL.
try {
// Split by semicolon but respect statements properly
// Note: This doesn't handle semicolons in strings, but our migrations are simple
$statements = array_filter(
$statements = array_values(array_filter(
array_map('trim', explode(';', $sql)),
function($stmt) {
// Remove comments and check if there's actual SQL
$cleaned = preg_replace('/--.*$/m', '', $stmt);
return !empty(trim($cleaned));
}
);
));
$resumeFrom = 0;
$progressStmt = $conn->prepare(
"SELECT last_statement_index FROM migration_progress WHERE filename = ?"
);
$progressStmt->bind_param('s', $filename);
$progressStmt->execute();
$progressRow = $progressStmt->get_result()->fetch_assoc();
$progressStmt->close();
if ($progressRow) {
$resumeFrom = (int)$progressRow['last_statement_index'] + 1;
echo "\n Resuming from statement " . ($resumeFrom + 1) . " of " . count($statements)
. " after a previous partial failure... ";
}
foreach ($statements as $index => $statement) {
if ($index < $resumeFrom) {
continue;
}
foreach ($statements as $statement) {
if (!$conn->query($statement)) {
// Some "errors" are acceptable (like "index already exists")
$error = $conn->error;
if (strpos($error, 'Duplicate key name') !== false ||
strpos($error, 'already exists') !== false) {
// Index already exists, that's fine
continue;
}
} else {
throw new Exception($error);
}
}
// Record the migration
// Record progress after every statement so a later run can
// resume from here even if a subsequent statement fails.
$upsert = $conn->prepare(
"INSERT INTO migration_progress (filename, last_statement_index) VALUES (?, ?)
ON DUPLICATE KEY UPDATE last_statement_index = VALUES(last_statement_index)"
);
$upsert->bind_param('si', $filename, $index);
$upsert->execute();
$upsert->close();
}
// Record the migration as fully complete and clear its progress marker
$stmt = $conn->prepare("INSERT INTO migrations (filename) VALUES (?)");
$stmt->bind_param('s', $filename);
if (!$stmt->execute()) {
throw new Exception("Could not record migration: " . $conn->error);
}
$conn->commit();
$clearProgress = $conn->prepare("DELETE FROM migration_progress WHERE filename = ?");
$clearProgress->bind_param('s', $filename);
$clearProgress->execute();
$clearProgress->close();
echo "OK\n";
$success++;
} catch (Exception $e) {
$conn->rollback();
// Nothing to roll back: every statement up to the failure already
// committed (DDL implicitly, everything else via autocommit). The
// progress marker recorded above reflects exactly how far this file
// got, so the next run will resume right after the last success.
echo "FAILED (" . $e->getMessage() . ")\n";
$failed++;
}
+1 -1
View File
@@ -38,7 +38,7 @@ class CommentModel
}
$placeholders = str_repeat('?,', count($usernames) - 1) . '?';
$sql = "SELECT user_id, username, display_name FROM users WHERE username IN ($placeholders)";
$sql = "SELECT user_id, username, display_name, is_admin, `groups` FROM users WHERE username IN ($placeholders)";
$stmt = $this->conn->prepare($sql);
$types = str_repeat('s', count($usernames));
+4 -1
View File
@@ -726,7 +726,10 @@ class TicketModel
$groupConditions = [];
foreach ($userGroups as $group) {
$groupConditions[] = "FIND_IN_SET(?, REPLACE(t.visibility_groups, ' ', ''))";
$params[] = $group;
// Strip spaces from the bound value too, matching the REPLACE()
// applied to the column, so a group name like "IT Support" is
// normalized the same way on both sides of the comparison.
$params[] = str_replace(' ', '', $group);
$types .= 's';
}
$conditions[] = "(t.visibility = 'internal' AND (" . implode(' OR ', $groupConditions) . "))";