Compare commits

...
Author SHA1 Message Date
jaredandClaude Opus 4.8 94ad84dae9 CI: pin actions/checkout to a commit SHA
Lint / Deploy (push) Successful in 4s
Lint / Notify on failure (push) Has been skipped
Lint / PHP (phpcs PSR-12) (push) Successful in 34s
Lint / JS (eslint) (push) Successful in 9s
Lint / PHP requirements (version + extensions) (push) Successful in 44s
Security / PHP Security (semgrep) (push) Successful in 2m48s
semgrep's github-actions-mutable-action-tag rule (now running, after the
pip install was fixed) flags actions/checkout@v3 as a mutable tag that
could be repointed upstream (supply-chain risk). Pin all four uses to the
SHA the v3 tag currently resolves to (v3.6.0), preserving behavior.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-30 14:53:38 -04:00
jaredandClaude Opus 4.8 99c840fce0 Fix logic bugs found in third multi-agent review
Security / PHP Security (semgrep) (push) Failing after 2m44s
Lint / Deploy (push) Successful in 8s
Lint / Notify on failure (push) Has been skipped
Lint / PHP (phpcs PSR-12) (push) Successful in 18s
Lint / JS (eslint) (push) Successful in 8s
Lint / PHP requirements (version + extensions) (push) Successful in 21s
Medium:
- create_ticket_api.php: environment tags were parsed with explode('][') which
  left brackets on the first/last tag so the whitelist never matched, dropping
  the env tag from the dedup hash — a [production] and [staging] issue with
  otherwise-identical components could collide onto one ticket. Use a
  bracket-aware regex.
- CommentModel::getThreadedCommentsPaged only fetched DIRECT children of root
  comments, so when pagination is active, nested replies at depth 2-3 vanished
  from the thread. Expand replies level-by-level (bounded to depth 3).
- StatsModel::getTicketsByAssignee ignored the visibility filter the rest of the
  stats apply, so a non-admin's "by assignee" widget counted (leaked) confidential
  tickets. Thread the same filter through.
- watch_ticket.php GET path returned watch state / watcher names / count for any
  ticket with no access check (the POST path checks it) — added canUserAccessTicket.
- dashboard.js kanban: every card rendered as P4 because the [class*="lt-p"]
  selector never matched the lt-badge-p1 class and the fallback didn't strip "P".
  Extract the digit directly.

Low:
- audit_log.php CSV: "Log ID" column was always blank ($log['log_id'] vs the real
  audit_id column). Use audit_id.
- check_duplicates.php: the graceful-degradation try/catch only covered the throw
  path; guard the false-return (non-exception mysqli) path too.
- notifications.php: owner-who-is-also-@mentioned got two notifications for one
  comment; drop the duplicate comment row when a mention covers the same comment.
- dashboard.js hover preview rendered "PP1" (doubled prefix); strip the leading P.
- markdown.js: code/inline-code restore used string replace, so $&, $$, $`, $' in
  user code were treated as replacement patterns; use a function replacer. Also
  removed an unused loop var.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-30 14:21:35 -04:00
jaredandClaude Opus 4.8 9941fd2dfa Address remaining review items: Synapse caching, cycle detection, cache/ratelimit/kanban
Security / PHP Security (semgrep) (push) Successful in 1m45s
Lint / Deploy (push) Successful in 3s
Lint / Notify on failure (push) Has been skipped
Lint / PHP (phpcs PSR-12) (push) Successful in 21s
Lint / JS (eslint) (push) Successful in 9s
Lint / PHP requirements (version + extensions) (push) Successful in 26s
- SynapseHelper: memoize username->Matrix-ID lookups per-request (incl. negative
  results) and add an overall time budget to resolveUsernames() plus a 2s connect
  timeout, so notifying N watchers with a slow/unreachable Synapse can't stall the
  request for N x 5s. (Chosen over async/queue per maintainer.)
- DependencyModel: fix cycle detection treating 'blocks' and 'blocked_by' as the
  same edge direction. They are inverse relationships (single row each, no mirror
  row), so the traversal now walks a unified precedence graph (blocks: ticket->
  depends_on; blocked_by: depends_on->ticket) and wouldCreateCycle normalizes the
  new edge's direction. Prevents both false-positive and missed cycles.
- CacheHelper: anchor prefix-delete to exact key boundaries (bare prefix or
  prefix + '_' + md5) so delete('workflow') can't wipe a 'workflow_rules' cache.
- RateLimitMiddleware: hold an exclusive flock across the per-IP counter's
  read-modify-write so concurrent requests can't both read N and write N+1
  (undercounting past the limit). Fails open if the file can't be locked.
- dashboard.js: kanban status update now uses lt.api.post (per no-raw-fetch
  convention) and reverts the card AND the optimistic column counts on failure
  (the old raw-fetch catch left the card moved without reverting).
- BulkOperationsModel: document that bulk_status/bulk_close intentionally bypass
  workflow transition validation (admin override, by design).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-30 12:55:50 -04:00
jaredandClaude Opus 4.8 e0e92e326a Quick-win fixes from second review
Security / PHP Security (semgrep) (push) Successful in 1m27s
Lint / Deploy (push) Successful in 4s
Lint / Notify on failure (push) Has been skipped
Lint / PHP (phpcs PSR-12) (push) Successful in 20s
Lint / JS (eslint) (push) Successful in 9s
Lint / PHP requirements (version + extensions) (push) Successful in 38s
- create_ticket_api.php: validate status (against TICKET_STATUSES) and
  priority (numeric 1-5). A non-numeric priority previously cast to 0 and
  escalated the ticket below P1 on the dedup/update path.
- manage_workflows.php: reject empty/invalid from_status/to_status on POST
  and PUT (must be valid ticket statuses) so the workflow table can't be
  populated with bogus transitions.
- TicketModel::getAllTickets: COUNT(*) OVER() rides on returned rows, so a
  page past the last row returned total/pages = 0. Fall back to a direct
  COUNT when an over-range page yields no rows, keeping pager math correct.
- DashboardView: stop double-escaping category/type/assigned active-filter
  labels (they were htmlspecialchars'd into the label and again at output,
  rendering R&D as R&amp;D); output escaping is retained.
- check_duplicates.php / NotificationHelper::notifyWatchers: wrap the DB
  lookups in try/catch so a failed prepare/query degrades gracefully
  (advisory dup-check returns none; best-effort watcher notify is skipped)
  instead of fataling the request. Works whether mysqli throws or returns
  false. (manage_* endpoints already have a top-level try/catch.)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-30 12:26:28 -04:00
20 changed files with 355 additions and 138 deletions
+3 -3
View File
@@ -11,7 +11,7 @@ jobs:
name: PHP (phpcs PSR-12)
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
- uses: actions/checkout@f43a0e5ff2bd294095638e18286ca9a3d1956744 # v3.6.0
- name: Install PHP and phpcs
run: |
@@ -27,7 +27,7 @@ jobs:
name: JS (eslint)
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
- uses: actions/checkout@f43a0e5ff2bd294095638e18286ca9a3d1956744 # v3.6.0
- name: Install ESLint
run: npm install --save-dev eslint@8
@@ -39,7 +39,7 @@ jobs:
name: PHP requirements (version + extensions)
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
- uses: actions/checkout@f43a0e5ff2bd294095638e18286ca9a3d1956744 # v3.6.0
- name: Install PHP with required extensions
run: |
+1 -1
View File
@@ -13,7 +13,7 @@ jobs:
name: PHP Security (semgrep)
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
- uses: actions/checkout@f43a0e5ff2bd294095638e18286ca9a3d1956744 # v3.6.0
- name: Install semgrep
run: |
+1 -1
View File
@@ -70,7 +70,7 @@ if ($_SERVER['REQUEST_METHOD'] === 'GET') {
}
fputcsv($output, [
$log['log_id'],
$log['audit_id'] ?? ($log['log_id'] ?? ''),
$log['created_at'],
$log['display_name'] ?? $log['username'] ?? 'N/A',
$log['action_type'],
+22 -5
View File
@@ -50,12 +50,29 @@ $sql = "SELECT ticket_id, title, status, priority, created_at
$types = "ss" . $visFilter['types'];
$params = array_merge([$searchTerm, $soundexTitle], $visFilter['params']);
$stmt = $conn->prepare($sql);
if (!empty($params)) {
$stmt->bind_param($types, ...$params);
// Duplicate detection is advisory (it must not block ticket creation), so on any
// DB error degrade gracefully to "no duplicates" rather than fataling the request.
// mysqli may throw (default exception mode) or return false depending on config.
try {
$stmt = $conn->prepare($sql);
if (!$stmt) {
throw new RuntimeException('prepare failed: ' . $conn->error);
}
if (!empty($params)) {
$stmt->bind_param($types, ...$params);
}
$stmt->execute();
$result = $stmt->get_result();
if ($result === false) {
// Non-exception mysqli mode: execute/get_result return false instead of
// throwing. Treat as a query failure so we don't fatal on $result below.
throw new RuntimeException('query failed: ' . $conn->error);
}
} catch (Throwable $e) {
error_log('check_duplicates: ' . $e->getMessage());
ResponseHelper::success(['duplicates' => []]);
}
$stmt->execute();
$result = $stmt->get_result();
while ($row = $result->fetch_assoc()) {
// Calculate similarity score
+18
View File
@@ -82,6 +82,15 @@ try {
case 'POST':
$data = json_decode(file_get_contents('php://input'), true);
$wfValid = $GLOBALS['config']['TICKET_STATUSES'] ?? ['Open', 'Pending', 'In Progress', 'Closed'];
if (
!in_array($data['from_status'] ?? '', $wfValid, true)
|| !in_array($data['to_status'] ?? '', $wfValid, true)
) {
http_response_code(400);
echo json_encode(['success' => false, 'error' => 'from_status and to_status must be valid ticket statuses']);
exit;
}
if (($data['from_status'] ?? '') === ($data['to_status'] ?? '')) {
http_response_code(400);
echo json_encode(['success' => false, 'error' => 'From Status and To Status cannot be the same']);
@@ -125,6 +134,15 @@ try {
$data = json_decode(file_get_contents('php://input'), true);
$wfValid = $GLOBALS['config']['TICKET_STATUSES'] ?? ['Open', 'Pending', 'In Progress', 'Closed'];
if (
!in_array($data['from_status'] ?? '', $wfValid, true)
|| !in_array($data['to_status'] ?? '', $wfValid, true)
) {
http_response_code(400);
echo json_encode(['success' => false, 'error' => 'from_status and to_status must be valid ticket statuses']);
exit;
}
if (($data['from_status'] ?? '') === ($data['to_status'] ?? '')) {
http_response_code(400);
echo json_encode(['success' => false, 'error' => 'From Status and To Status cannot be the same']);
+17
View File
@@ -175,6 +175,23 @@ $stmt->execute();
$mentionRows = $stmt->get_result()->fetch_all(MYSQLI_ASSOC);
$stmt->close();
// If the user owns/watches a ticket AND was @mentioned in the same comment, the
// comment query and the mention query both produce a row for it. Prefer the more
// specific mention and drop the duplicate comment notification for that comment.
$mentionCommentIds = [];
foreach ($mentionRows as $mr) {
$md = json_decode($mr['details'] ?? '{}', true) ?? [];
if (!empty($md['comment_id'])) {
$mentionCommentIds[(int)$md['comment_id']] = true;
}
}
if (!empty($mentionCommentIds)) {
$commentRows = array_filter(
$commentRows,
fn($cr) => !isset($mentionCommentIds[(int)($cr['entity_id'] ?? 0)])
);
}
// Merge, deduplicate by log_id, sort by created_at desc
$all = [];
$seen = [];
+11
View File
@@ -78,6 +78,17 @@ if ($ticketId <= 0) {
exit;
}
// Enforce ticket visibility before returning watch state / watcher names, so a
// restricted ticket's watcher list and count aren't disclosed (the POST path
// already checks this).
$ticketModel = new TicketModel($conn);
$ticket = $ticketModel->getTicketById($ticketId);
if (!$ticket || !$ticketModel->canUserAccessTicket($ticket, $currentUser)) {
http_response_code(404);
echo json_encode(['success' => false, 'error' => 'Ticket not found']);
exit;
}
$watchingStmt = $conn->prepare(
"SELECT COUNT(*) as cnt FROM ticket_watchers WHERE ticket_id = ? AND user_id = ?"
);
+31 -26
View File
@@ -1142,8 +1142,10 @@ function populateKanbanCards() {
if (cells.length < 6) return;
const ticketId = cells[0 + offset]?.querySelector('.ticket-link')?.textContent.trim() || '';
const priorityEl = cells[1 + offset]?.querySelector('[class*="lt-p"]');
const priority = priorityEl ? priorityEl.textContent.trim().replace('P','') : cells[1 + offset]?.textContent.trim() || '4';
// The priority cell renders a "P1".."P5" badge; extract just the digit.
// (The old [class*="lt-p"] selector never matched the lt-badge-p1 class, so
// every card fell back to P4 regardless of real priority.)
const priority = (cells[1 + offset]?.textContent.trim() || '').replace(/[^0-9]/g, '') || '4';
const title = cells[2 + offset]?.textContent.trim() || '';
const category = cells[3 + offset]?.textContent.trim() || '';
const statusEl = cells[5 + offset]?.querySelector('.lt-status');
@@ -1219,29 +1221,30 @@ function populateKanbanCards() {
if (dec) dec.textContent = '(' + Math.max(0, (parseInt(dec.textContent.replace(/\D/g,''),10)||1) - 1) + ')';
if (inc) inc.textContent = '(' + ((parseInt(inc.textContent.replace(/\D/g,''),10)||0) + 1) + ')';
// POST status update
fetch('/api/update_ticket.php', {
method: 'POST',
credentials: 'same-origin',
headers: { 'Content-Type': 'application/json', 'X-CSRF-Token': window.CSRF_TOKEN || '' },
body: JSON.stringify({ ticket_id: String(ticketId), status: newStatus })
})
.then(r => r.json())
.then(data => {
if (data.success) {
lt.toast.success('Ticket #' + ticketId + ' → ' + newStatus, 2500);
movedCard.dataset.status = newStatus;
} else {
lt.toast.error('Status update failed: ' + (data.error || 'Unknown error'));
// Revert: put card back in original column
const origCol = document.getElementById(Object.keys(colStatusMap).find(k => colStatusMap[k] === oldStatus));
if (origCol) origCol.appendChild(movedCard);
movedCard.dataset.status = oldStatus;
}
})
.catch(() => {
lt.toast.error('Network error — status not saved');
});
// Revert the card to its original column and undo the optimistic counts.
const revert = function () {
const origCol = document.getElementById(Object.keys(colStatusMap).find(k => colStatusMap[k] === oldStatus));
if (origCol) origCol.appendChild(movedCard);
movedCard.dataset.status = oldStatus;
if (dec) dec.textContent = '(' + ((parseInt(dec.textContent.replace(/\D/g, ''), 10) || 0) + 1) + ')';
if (inc) inc.textContent = '(' + Math.max(0, (parseInt(inc.textContent.replace(/\D/g, ''), 10) || 1) - 1) + ')';
};
// POST status update via the shared wrapper (adds CSRF + JSON, throws on non-2xx)
lt.api.post('/api/update_ticket.php', { ticket_id: String(ticketId), status: newStatus })
.then(function (data) {
if (data && data.success) {
lt.toast.success('Ticket #' + ticketId + ' → ' + newStatus, 2500);
movedCard.dataset.status = newStatus;
} else {
lt.toast.error('Status update failed: ' + ((data && data.error) || 'Unknown error'));
revert();
}
})
.catch(function () {
lt.toast.error('Status update failed — reverting');
revert();
});
}
Object.keys(columns).forEach(status => {
@@ -1314,7 +1317,9 @@ function showTicketPreview(event) {
const offset = isAdmin ? 1 : 0;
const ticketId = link.textContent.trim();
const priority = cells[1 + offset]?.textContent.trim() || '';
// Cell text is already "P1".."P5"; strip the leading P so the template's
// `P${priority}` doesn't render "PP1".
const priority = (cells[1 + offset]?.textContent.trim() || '').replace(/^P/i, '');
const title = cells[2 + offset]?.textContent.trim() || '';
const category = cells[3 + offset]?.textContent.trim() || '';
const type = cells[4 + offset]?.textContent.trim() || '';
+6 -4
View File
@@ -142,12 +142,14 @@ function parseMarkdown(markdown) {
html = html.replace(/ \n/g, '<br>');
html = html.replace(/\n\n/g, '</p><p>');
// Restore code blocks and inline code
// Restore code blocks and inline code. Use a function replacer so '$'
// sequences in user code (e.g. $&, $$, $`, $') are inserted literally rather
// than interpreted as String.replace replacement patterns.
codeBlocks.forEach((block, i) => {
html = html.replace('%%CODEBLOCK' + i + '%%', block);
html = html.replace('%%CODEBLOCK' + i + '%%', () => block);
});
inlineCodes.forEach((code, i) => {
html = html.replace('%%INLINECODE' + i + '%%', code);
html = html.replace('%%INLINECODE' + i + '%%', () => code);
});
// Restore footnote reference placeholders
@@ -164,7 +166,7 @@ function parseMarkdown(markdown) {
// Append footnote definitions block
if (footnoteOrder.length) {
html += '<hr class="fn-hr"><ol class="fn-list">';
footnoteOrder.forEach(function(label, i) {
footnoteOrder.forEach(function(label) {
html += '<li id="fn-' + fnSlug(label) + '" class="fn-item">' +
parseMarkdown(footnotes[label]).replace(/<\/?p>/g, '') +
' <a href="#fnref-' + fnSlug(label) + '" class="fn-back">&#x21A9;</a></li>';
+27 -4
View File
@@ -195,10 +195,17 @@ function generateTicketHash($data)
'source_type' => $sourceType,
'issue_category' => $issueCategory,
'issue_subtype' => $issueSubtype,
'environment_tags' => array_values(array_filter(
explode('][', $title),
fn($tag) => in_array($tag, ['production', 'development', 'staging', 'single-node', 'cluster-wide'])
)),
'environment_tags' => (function () use ($title) {
// Extract each [bracketed] tag, then keep the known environment ones.
// (explode('][') leaves brackets stuck to the first/last tag, so e.g.
// "[production] ..." never matched and the env tag was dropped from the
// dedup hash — letting prod and staging issues collide onto one ticket.)
preg_match_all('/\[([^\]]+)\]/', $title, $m);
return array_values(array_filter(
$m[1],
fn($tag) => in_array($tag, ['production', 'development', 'staging', 'single-node', 'cluster-wide'], true)
));
})(),
];
// Manual tickets should be unique by title (so different software installs don't collide)
@@ -231,6 +238,22 @@ $priority = $data['priority'] ?? '4';
$category = (string)($data['category'] ?? 'General');
$type = (string)($data['type'] ?? 'Issue');
// Validate externally-supplied status and priority. (category/type are free-form
// in this schema.) A non-numeric priority would otherwise cast to 0 and escalate
// the ticket below P1 on the dedup/update path.
$validStatuses = $GLOBALS['config']['TICKET_STATUSES'] ?? ['Open', 'Pending', 'In Progress', 'Closed'];
if (!in_array($status, $validStatuses, true)) {
http_response_code(400);
echo json_encode(['success' => false, 'error' => 'Invalid status']);
exit;
}
if (!is_numeric($priority) || (int)$priority < 1 || (int)$priority > 5) {
http_response_code(400);
echo json_encode(['success' => false, 'error' => 'Invalid priority (must be 1-5)']);
exit;
}
$priority = (int)$priority;
$ticketHash = generateTicketHash($data);
$auditLog = new AuditLogModel($conn);
+13 -6
View File
@@ -125,16 +125,23 @@ class CacheHelper
return !file_exists($filePath) || @unlink($filePath);
}
// Delete all files with this prefix
$pattern = self::getCacheDir() . '/' . preg_replace('/[^a-zA-Z0-9_]/', '_', $prefix) . '*.json';
$files = glob($pattern);
// Delete all entries for this prefix. A key is either the bare prefix or
// prefix + '_' + md5(identifier) (32 hex chars, see makeKey). Match exactly
// that so a prefix can't clobber a different prefix that merely shares a
// leading substring — e.g. delete('workflow') must not wipe 'workflow_rules'.
$safePrefix = preg_replace('/[^a-zA-Z0-9_]/', '_', $prefix);
$keyRegex = '/^' . preg_quote($safePrefix, '/') . '(_[0-9a-f]{32})?$/';
$files = glob(self::getCacheDir() . '/' . $safePrefix . '*.json') ?: [];
foreach ($files as $file) {
@unlink($file);
if (preg_match($keyRegex, basename($file, '.json'))) {
@unlink($file);
}
}
// Clear memory cache entries with this prefix
// Clear matching memory cache entries
foreach (array_keys(self::$memoryCache) as $key) {
if (strpos($key, $prefix) === 0) {
if (preg_match($keyRegex, $key)) {
unset(self::$memoryCache[$key]);
}
}
+31 -25
View File
@@ -164,32 +164,38 @@ class NotificationHelper
return;
}
// Fetch watcher usernames, excluding the actor so they don't notify themselves
if ($excludeUserId !== null) {
$sql = "SELECT u.username FROM ticket_watchers tw JOIN users u ON tw.user_id = u.user_id WHERE tw.ticket_id = ? AND tw.user_id != ?";
$stmt = $conn->prepare($sql);
} else {
$sql = "SELECT u.username FROM ticket_watchers tw JOIN users u ON tw.user_id = u.user_id WHERE tw.ticket_id = ?";
$stmt = $conn->prepare($sql);
}
// Notifications are best-effort; if the watchers table is absent or the
// statement fails to prepare, skip silently rather than fataling the
// request that already committed its DB change.
if (!$stmt) {
return;
}
if ($excludeUserId !== null) {
$stmt->bind_param("ii", $ticketId, $excludeUserId);
} else {
$stmt->bind_param("i", $ticketId);
}
$stmt->execute();
$result = $stmt->get_result();
$stmt->close();
// Fetch watcher usernames, excluding the actor so they don't notify
// themselves. Notifications are best-effort: if the watchers table is
// absent or the query fails, skip silently rather than fataling the
// request that already committed its DB change. mysqli may either throw
// (default exception mode) or return false, so handle both.
$usernames = [];
while ($row = $result->fetch_assoc()) {
$usernames[] = $row['username'];
try {
if ($excludeUserId !== null) {
$sql = "SELECT u.username FROM ticket_watchers tw JOIN users u ON tw.user_id = u.user_id WHERE tw.ticket_id = ? AND tw.user_id != ?";
$stmt = $conn->prepare($sql);
} else {
$sql = "SELECT u.username FROM ticket_watchers tw JOIN users u ON tw.user_id = u.user_id WHERE tw.ticket_id = ?";
$stmt = $conn->prepare($sql);
}
if (!$stmt) {
return;
}
if ($excludeUserId !== null) {
$stmt->bind_param("ii", $ticketId, $excludeUserId);
} else {
$stmt->bind_param("i", $ticketId);
}
$stmt->execute();
$result = $stmt->get_result();
$stmt->close();
while ($row = $result->fetch_assoc()) {
$usernames[] = $row['username'];
}
} catch (\Throwable $e) {
error_log('NotificationHelper::notifyWatchers watcher lookup failed: ' . $e->getMessage());
return;
}
if (empty($usernames)) {
+33 -12
View File
@@ -4,8 +4,9 @@
* SynapseHelper
*
* Resolves local (SSO) usernames Matrix user IDs by querying the
* Synapse Admin REST API directly. No caching every call is live
* so results never go stale.
* Synapse Admin REST API directly. Results are memoized per-request (not
* across requests, so they don't go stale between requests), and a batch
* resolve has an overall time budget to bound request latency.
*
* Required config (.env) keys:
* MATRIX_DOMAIN e.g. matrix.lotusguild.org
@@ -14,6 +15,12 @@
*/
class SynapseHelper
{
/** Per-request memo of username => Matrix ID|null, so repeat watchers are free. */
private static array $cache = [];
/** Total wall-clock budget (seconds) for a single resolveUsernames() batch. */
private const RESOLVE_BUDGET_SECONDS = 5;
/**
* Resolve a local SSO username to its Matrix user ID.
*
@@ -29,6 +36,11 @@ class SynapseHelper
*/
public static function resolveUsername(string $username): ?string
{
// Serve from the per-request cache when we've already looked this up.
if (array_key_exists($username, self::$cache)) {
return self::$cache[$username];
}
$baseUrl = $GLOBALS['config']['SYNAPSE_ADMIN_URL'] ?? null;
$token = $GLOBALS['config']['SYNAPSE_ADMIN_TOKEN'] ?? null;
$domain = $GLOBALS['config']['MATRIX_DOMAIN'] ?? null;
@@ -49,6 +61,7 @@ class SynapseHelper
'Accept: application/json',
]);
curl_setopt($ch, CURLOPT_RETURNTRANSFER, true);
curl_setopt($ch, CURLOPT_CONNECTTIMEOUT, 2); // fail fast when Synapse is unreachable
curl_setopt($ch, CURLOPT_TIMEOUT, 5);
$body = curl_exec($ch);
@@ -56,25 +69,24 @@ class SynapseHelper
$curlError = curl_error($ch);
curl_close($ch);
$resolved = null;
if ($curlError) {
error_log("SynapseHelper: cURL error resolving '{$username}': {$curlError}");
return null;
}
if ($httpCode === 200) {
} elseif ($httpCode === 200) {
$data = json_decode($body, true);
// Confirm the response contains the name we expect
if (!empty($data['name'])) {
return $data['name']; // e.g. "@jared:matrix.lotusguild.org"
$resolved = $data['name']; // e.g. "@jared:matrix.lotusguild.org"
}
}
// 404 = user not found in Synapse; other codes = error
if ($httpCode !== 404) {
} elseif ($httpCode !== 404) {
// 404 = user not found in Synapse; other codes = error
error_log("SynapseHelper: unexpected HTTP {$httpCode} resolving '{$username}'");
}
return null;
// Memoize for the rest of this request (including negative results, so a
// missing/unreachable user isn't retried within the same request).
self::$cache[$username] = $resolved;
return $resolved;
}
/**
@@ -87,7 +99,16 @@ class SynapseHelper
public static function resolveUsernames(array $usernames): array
{
$ids = [];
$deadline = microtime(true) + self::RESOLVE_BUDGET_SECONDS;
foreach ($usernames as $username) {
// Cached lookups are free and always allowed; for uncached ones, stop
// making live calls once the batch budget is spent so a slow/unreachable
// Synapse can't stall the request for N × per-call timeout.
$cached = array_key_exists($username, self::$cache);
if (!$cached && microtime(true) >= $deadline) {
error_log('SynapseHelper: resolve budget exhausted; skipping remaining lookups');
break;
}
$id = self::resolveUsername($username);
if ($id !== null) {
$ids[] = $id;
+28 -13
View File
@@ -84,28 +84,43 @@ class RateLimitMiddleware
$ipHash = hash('sha256', $ip . '_' . $type);
$filePath = self::getRateLimitDir() . '/' . $ipHash . '.json';
// Load existing rate data
// Hold an exclusive lock across the whole read-modify-write so concurrent
// requests from the same IP can't both read the same count and each write
// count+1 (which would undercount and let the limit be exceeded).
$fh = @fopen($filePath, 'c+');
if ($fh === false) {
// Can't open the counter file — fail open (don't block legitimate traffic).
return true;
}
if (!flock($fh, LOCK_EX)) {
fclose($fh);
return true;
}
$content = stream_get_contents($fh);
$rateData = ['count' => 0, 'window_start' => $now];
if (file_exists($filePath)) {
$content = @file_get_contents($filePath);
if ($content !== false) {
$decoded = json_decode($content, true);
if (is_array($decoded)) {
$rateData = $decoded;
}
if ($content !== false && $content !== '') {
$decoded = json_decode($content, true);
if (is_array($decoded)) {
$rateData = $decoded;
}
}
// Check if window has expired
if ($now - $rateData['window_start'] >= self::WINDOW_SECONDS) {
// Reset when the window has expired
if ($now - ($rateData['window_start'] ?? $now) >= self::WINDOW_SECONDS) {
$rateData = ['count' => 0, 'window_start' => $now];
}
// Increment count
$rateData['count']++;
// Save updated data
@file_put_contents($filePath, json_encode($rateData), LOCK_EX);
// Rewrite the file in place while still holding the lock
rewind($fh);
ftruncate($fh, 0);
fwrite($fh, json_encode($rateData));
fflush($fh);
flock($fh, LOCK_UN);
fclose($fh);
// Check if over limit
return $rateData['count'] <= $limit;
+5
View File
@@ -104,6 +104,11 @@ class BulkOperationsModel
$success = false;
try {
// NOTE: bulk_status / bulk_close intentionally do NOT run
// WorkflowModel::isTransitionAllowed(). Bulk operations are an
// admin-only escape hatch for forcing ticket states (e.g. mass
// re-opening), so they bypass the workflow transition rules that
// the single-ticket update path enforces. This is by design.
switch ($operation['operation_type']) {
case 'bulk_close':
// Get current ticket from pre-loaded batch
+34 -20
View File
@@ -176,27 +176,41 @@ class CommentModel
return [];
}
// All replies for these root comments (up to 3 levels deep)
$placeholders = implode(',', array_fill(0, count($rootIds), '?'));
$replySql = "SELECT tc.*, u.display_name, u.username
FROM ticket_comments tc
LEFT JOIN users u ON tc.user_id = u.user_id
WHERE tc.ticket_id = ?
AND tc.parent_comment_id IN ($placeholders)
AND tc.parent_comment_id IS NOT NULL
ORDER BY tc.created_at ASC";
$replyStmt = $this->conn->prepare($replySql);
$types = 'i' . str_repeat('i', count($rootIds));
$replyStmt->bind_param($types, $ticketId, ...$rootIds);
$replyStmt->execute();
$replyResult = $replyStmt->get_result();
$replyStmt->close();
// Load replies level-by-level under this page's roots. A single
// "parent_comment_id IN (rootIds)" only fetches DIRECT children, so
// grandchildren/great-grandchildren (addComment allows up to depth 3)
// would be missing from the map and dropped by buildCommentThread.
// Expand iteratively until no new replies (bounded by max depth 3).
$parentIds = $rootIds;
$depth = 0;
while (!empty($parentIds) && $depth < 3) {
$placeholders = implode(',', array_fill(0, count($parentIds), '?'));
$replySql = "SELECT tc.*, u.display_name, u.username
FROM ticket_comments tc
LEFT JOIN users u ON tc.user_id = u.user_id
WHERE tc.ticket_id = ?
AND tc.parent_comment_id IN ($placeholders)
ORDER BY tc.created_at ASC";
$replyStmt = $this->conn->prepare($replySql);
$types = 'i' . str_repeat('i', count($parentIds));
$replyStmt->bind_param($types, $ticketId, ...$parentIds);
$replyStmt->execute();
$replyResult = $replyStmt->get_result();
$replyStmt->close();
while ($row = $replyResult->fetch_assoc()) {
$row['display_name_formatted'] = $row['display_name'] ?: ($row['user_name'] ?? 'Unknown User');
$row['replies'] = [];
$row['thread_depth'] = $row['thread_depth'] ?? 1;
$commentMap[$row['comment_id']] = $row;
$nextParentIds = [];
while ($row = $replyResult->fetch_assoc()) {
if (isset($commentMap[$row['comment_id']])) {
continue; // guard against cycles / duplicates
}
$row['display_name_formatted'] = $row['display_name'] ?: ($row['user_name'] ?? 'Unknown User');
$row['replies'] = [];
$row['thread_depth'] = $depth + 1;
$commentMap[$row['comment_id']] = $row;
$nextParentIds[] = $row['comment_id'];
}
$parentIds = $nextParentIds;
$depth++;
}
$rootComments = [];
+25 -7
View File
@@ -190,14 +190,25 @@ class DependencyModel
*/
private function wouldCreateCycle($ticketId, $dependsOnId, $type): bool
{
// Only check for cycles in blocking relationships
// Only blocking relationships impose an ordering that can form a cycle.
if (!in_array($type, ['blocks', 'blocked_by'])) {
return false;
}
// Check if dependsOnId already has ticketId in its dependency chain
// Normalize the new row to a precedence edge "from must finish before to":
// (t, d, 'blocks') => t blocks d => edge t -> d
// (t, d, 'blocked_by') => t blocked_by d => edge d -> t
if ($type === 'blocks') {
$from = $ticketId;
$to = $dependsOnId;
} else { // blocked_by
$from = $dependsOnId;
$to = $ticketId;
}
// Adding edge from->to creates a cycle iff a path to ->* from already exists.
$visited = [];
return $this->hasDependencyPath($dependsOnId, $ticketId, $visited, 0);
return $this->hasDependencyPath($to, $from, $visited, 0);
}
/**
@@ -236,15 +247,22 @@ class DependencyModel
$visited[] = $source;
$sql = "SELECT depends_on_id FROM ticket_dependencies
WHERE ticket_id = ? AND dependency_type IN ('blocks', 'blocked_by')";
// Walk the unified precedence graph forward from $source. Both directions
// of expression contribute an outgoing edge "$source must finish before X":
// blocks rows where ticket_id=$source -> X = depends_on_id
// blocked_by rows where depends_on_id=$source -> X = ticket_id
$sql = "SELECT depends_on_id AS next_id FROM ticket_dependencies
WHERE ticket_id = ? AND dependency_type = 'blocks'
UNION
SELECT ticket_id AS next_id FROM ticket_dependencies
WHERE depends_on_id = ? AND dependency_type = 'blocked_by'";
$stmt = $this->conn->prepare($sql);
$stmt->bind_param("s", $source);
$stmt->bind_param("ss", $source, $source);
$stmt->execute();
$result = $stmt->get_result();
while ($row = $result->fetch_assoc()) {
if ($this->hasDependencyPath($row['depends_on_id'], $target, $visited, $depth + 1)) {
if ($this->hasDependencyPath($row['next_id'], $target, $visited, $depth + 1)) {
$stmt->close();
return true;
}
+23 -8
View File
@@ -28,8 +28,14 @@ class StatsModel
/**
* Get tickets by assignee (top 5)
*/
public function getTicketsByAssignee(int $limit = 8): array
public function getTicketsByAssignee(int $limit = 8, array $visFilter = []): array
{
// Apply the same visibility filter as the rest of the stats so a non-admin's
// assignee widget doesn't count (and thereby leak) confidential tickets.
$visSQL = $visFilter['sql'] ?? '';
$visParams = $visFilter['params'] ?? [];
$visTypes = $visFilter['types'] ?? '';
$sql = "SELECT
u.user_id,
u.display_name,
@@ -37,12 +43,20 @@ class StatsModel
COUNT(t.ticket_id) as open_count
FROM tickets t
LEFT JOIN users u ON t.assigned_to = u.user_id
WHERE t.status != 'Closed' AND t.assigned_to IS NOT NULL
GROUP BY t.assigned_to
ORDER BY open_count DESC
LIMIT ?";
WHERE t.status != 'Closed' AND t.assigned_to IS NOT NULL";
if ($visSQL !== '') {
$sql .= " AND ($visSQL)";
}
$sql .= " GROUP BY t.assigned_to
ORDER BY open_count DESC
LIMIT ?";
$params = $visParams;
$params[] = $limit;
$types = $visTypes . 'i';
$stmt = $this->conn->prepare($sql);
$stmt->bind_param('i', $limit);
$stmt->bind_param($types, ...$params);
$stmt->execute();
$result = $stmt->get_result();
$data = [];
@@ -173,8 +187,9 @@ class StatsModel
// Sort priority keys
ksort($byPriority);
// Query 3: Get assignee stats (requires JOIN, kept separate)
$byAssignee = $this->getTicketsByAssignee();
// Query 3: Get assignee stats (requires JOIN, kept separate). Pass the same
// visibility filter so confidential tickets aren't counted for non-admins.
$byAssignee = $this->getTicketsByAssignee(8, $visFilter);
return [
'open_tickets' => (int)($counts['open_tickets'] ?? 0),
+23
View File
@@ -208,6 +208,11 @@ class TicketModel
ORDER BY $sortExpression $sortDirection
LIMIT ? OFFSET ?";
// Keep a copy of the filter params (without LIMIT/OFFSET) for the
// fallback COUNT below.
$countParams = $params;
$countParamTypes = $paramTypes;
$params[] = $limit;
$params[] = $offset;
$paramTypes .= 'ii';
@@ -228,6 +233,24 @@ class TicketModel
}
$stmt->close();
// COUNT(*) OVER() rides on returned rows, so a page past the last row
// yields zero rows and a bogus total of 0. Fall back to a direct COUNT
// so the total/pages stay correct for stale or over-range page links.
if ($totalTickets === 0 && $offset > 0) {
$countSql = "SELECT COUNT(*) AS c
FROM tickets t
LEFT JOIN users u_created ON t.created_by = u_created.user_id
LEFT JOIN users u_assigned ON t.assigned_to = u_assigned.user_id
$whereClause";
$countStmt = $this->conn->prepare($countSql);
if (!empty($countParams)) {
$countStmt->bind_param($countParamTypes, ...$countParams);
}
$countStmt->execute();
$totalTickets = (int)($countStmt->get_result()->fetch_assoc()['c'] ?? 0);
$countStmt->close();
}
return [
'tickets' => $tickets,
'total' => $totalTickets,
+3 -3
View File
@@ -48,14 +48,14 @@ if (!empty($_GET['priority'])) {
}
}
if (!empty($_GET['category'])) {
$activeFilters[] = ['type' => 'category', 'value' => $_GET['category'], 'label' => 'Category: ' . htmlspecialchars($_GET['category'])];
$activeFilters[] = ['type' => 'category', 'value' => $_GET['category'], 'label' => 'Category: ' . $_GET['category']];
}
if (!empty($_GET['type'])) {
$activeFilters[] = ['type' => 'type', 'value' => $_GET['type'], 'label' => 'Type: ' . htmlspecialchars($_GET['type'])];
$activeFilters[] = ['type' => 'type', 'value' => $_GET['type'], 'label' => 'Type: ' . $_GET['type']];
}
if (!empty($_GET['assigned_to'])) {
$label = match ($_GET['assigned_to']) {
'unassigned' => 'Unassigned', 'me' => 'Me', default => 'User #' . htmlspecialchars($_GET['assigned_to'])
'unassigned' => 'Unassigned', 'me' => 'Me', default => 'User #' . $_GET['assigned_to']
};
$activeFilters[] = ['type' => 'assigned_to', 'value' => $_GET['assigned_to'], 'label' => 'Assigned: ' . $label];
}