Company Boundaries October 2026 - edmozley/freeitsm GitHub Wiki

Security: company boundaries — seven places they did not hold

Fixed in 3.15.1 · Reported privately by a security researcher, who asked not to be named · Present in every release up to and including 3.15.0 · Modules: Tickets, REST API, Messaging, Search, Tasks, Workflow, Watchtower, Knowledge

The short version

This page only matters if you run more than one company in one FreeITSM. On a single-company install there is no boundary to cross, and nothing here applies.

FreeITSM keeps each company's tickets, people, tasks and counts apart, and lets an analyst — or an API key — be limited to some companies. The same private report found seven places where that limit did not hold. Each needs an analyst, an API key, a workflow rule or a requester who messages two companies. Most leaked a little — names, email addresses, search snippets, counts — or filed something under the wrong company.

	What	Who could do it	What they got
1	Requesters from another company, through a ticket	a company-limited API key; an analyst limited to some companies	any company's requesters' names and email addresses; new requesters created in a company they cannot reach
2	WhatsApp threading across companies	anyone who messages two companies' WhatsApp numbers	their message filed on the other company's open ticket
3	A moved ticket left its search entries and tasks behind	nobody acting on purpose — any analyst in the old company	search snippets of the moved ticket's messages and internal notes; its tasks
4	Workflow tickets had no company	nobody acting on purpose — any "Create a ticket" workflow rule	every workflow ticket filed in triage, whoever it was for
5	An analyst with no company saw the Default company	an analyst who has been given no company	the Default company's tickets, tasks, changes and search results
6	Watchtower counted every company	any analyst limited to some companies	other companies' ticket, change, task and asset counts
7	A failed lookup meant "all companies"	nobody on purpose — a database error at the wrong moment	for that request, every company

Upgrading is the fix for all seven.

The pattern

In every one of these, the right check already existed, one step away. POST /users checked a named company and not an inferred one. The New ticket form checked a chosen requester and not a typed one. Telegram, Mattermost and Teams threaded within their channel; WhatsApp did not. The "All companies" view failed closed; the single-company view did not. The form-submission branch of a workflow set a company; the branch beside it did not. Watchtower's Knowledge card was scoped by company — and the comment beside it said the rest were not. Nobody had to invent anything. Each fix is the neighbouring check, applied to the case next to it.

1. Requesters from another company, through a ticket

What was wrong

POST /api/v1/users checked a company-limited key's limit when you named a company, and not when the company came from the address's domain. That half is written up in four smaller gaps; it was fixed in api/v1/resources/users.php by giving the inferred company the same check:

if ($tenantId !== null && !apiKeyCanAccessTenant($conn, $apiKey, (int)$tenantId)) {
    apiError(403, 'forbidden', "This address belongs to a company this API key cannot reach. Pass a 'tenant_id' it can, or null for triage.");
}

The sweep afterwards asked where else a requester is chosen or created — and the answer was raising a ticket.

POST /api/v1/tickets never checked user_id. The endpoint accepts an existing requester by id. It checked the ticket's company against the key, but passed the id straight to TicketsService::createTicket() — and the service says, in capitals, that it will not check it:

// ⚠️ THE CALLER MUST HAVE AUTHORISED THIS ID ALREADY. The service takes
// it on trust, exactly as it takes tenantId on trust — see
// api/tickets/create_ticket.php, which calls analystCanAccessUser()
// before getting here. A scoped picker list is not a check: the id
// arrives in a request body and can be edited.

The analyst New ticket form did that. The REST adapter did not. So a key limited to company A could raise a ticket naming any requester by id, and the 201 response — which includes the requester's name and email address — handed them back. Ids are sequential: a loop listed every company's contacts. (Each try also left a real ticket behind in company A.)

And a new requester's company was not checked, in two places. When the requester's address is one nobody has yet, raising the ticket creates them, in the company their email domain maps to. Neither POST /api/v1/tickets nor the analyst New ticket form (api/tickets/create_ticket.php) asked whether the caller could reach that company. An analyst limited to company A could create a person in company B by typing their address.

The fix

api/v1/resources/tickets.php checks both, before the ticket is raised:

if (isset($body['user_id']) && (int)$body['user_id'] > 0) {
    if (!apiKeyCanAccessTenantRow($conn, $apiKey, 'users', (int)$body['user_id'])) {
        apiError(422, 'invalid_field', 'That requester no longer exists.');   // same words as a missing id
    }
} else {
    $known = $conn->prepare("SELECT id FROM users WHERE email = ?");
    $known->execute([$requesterEmail]);
    if ($known->fetchColumn() === false) {
        $newTenant = resolveTenantForNewUser($conn, $requesterEmail);
        if ($newTenant !== null && !apiKeyCanAccessTenant($conn, $apiKey, (int)$newTenant)) {
            apiError(403, 'forbidden', "This address belongs to a company this API key cannot reach, so the requester cannot be created.");
        }
    }
}

🔑 An id the key cannot reach gets the same words as an id that does not exist — otherwise the error itself would confirm which ids are real people.

api/tickets/create_ticket.php gets the check api/tickets/save_user.php already made when an analyst adds a person, including its isMultiTenant() short-circuit — on a single-company install there are no company-access rows, so without it every analyst would be refused:

if ($userId === null && $fromEmail !== '' && isMultiTenant($conn)) {
    $known = $conn->prepare("SELECT id FROM users WHERE email = ?");
    $known->execute([strtolower(trim($fromEmail))]);
    if ($known->fetchColumn() === false) {
        $newTenant = resolveTenantForNewUser($conn, $fromEmail);
        if ($newTenant !== null && !analystCanAccessTenant($conn, $analystId, (int)$newTenant)) {
            echo json_encode(['success' => false, 'error' => 'You do not have access to that company']);
            exit;
        }
    }
}

2. WhatsApp threading across companies

What was wrong

A chat message is added to the sender's open ticket, if they have one. findOpenChannelTicket() in includes/messaging/ingest.php had been taught, channel type by channel type, to look only within the channel the message arrived on:

$byBot = (in_array($channelType, ['telegram', 'mattermost', 'teams'], true) && $channelId !== null);

WhatsApp was not on the list, so it matched by phone number across every channel — and a phone number is the same whichever company's WhatsApp number it messages. Someone who contacted two companies through one FreeITSM could have a message filed on the other company's open ticket. The full write-up is in four smaller gaps.

The fix

The list is gone. Every channel type matches within its own channel:

$byBot = ($channelId !== null);

Adding WhatsApp to the list would have fixed WhatsApp, and left the next channel type to remember to join it — which is exactly how WhatsApp was missed. Every channel message has recorded its channel (emails.channel_id) since the first channel shipped, so conversations already open keep threading.

3. A moved ticket left its search entries and tasks behind

What was wrong

An analyst can move a ticket to another company, and routing a ticket out of triage files it — and every other triage ticket from the same sender or domain — under a company. Both wrote one column:

$conn->prepare("UPDATE tickets SET tenant_id = ?, updated_datetime = UTC_TIMESTAMP() WHERE id = ?")
     ->execute([$targetId, $ticketId]);

Two things did not follow:

Search. Each ticket's search entries carry a copy of its company, taken when it was indexed. Nothing re-indexed on a move, so the old company kept finding the ticket in search — with snippets of its messages and internal notes — until a later reply happened to re-index it. The new company did not find it.
Tasks. Tasks raised from the ticket, and their subtasks, have a company of their own. They stayed in the old company, visible there.

The fix

Two parts, on purpose.

Search judges a ticket by its company now, not its copy. searchCorpusQuery() in includes/search/search.php — the one query every search goes through — adds the ticket's live company to the filter:

if (!empty($scope['tenant_id'])) {
    $delSql .= " AND (sd.ticket_id IS NULL OR t.tenant_id = ?"
             . (!empty($scope['include_default']) ? " OR t.tenant_id IS NULL" : '') . ")";
    $liveParams[] = (int)$scope['tenant_id'];
}

This is the part that makes the boundary hold. It covers every path that changes a ticket's company — the ones fixed below, and any written later that forgets to call anything.

Then the ticket's dependants follow it. New ticketsFollowCompanyMove() in includes/tenancy.php moves the ticket's tasks, and their subtasks (up to ten levels down), to the new company, and re-indexes the ticket so the new company finds it. It is called from all three move paths:

api/tickets/move_ticket_to_company.php — the Move button;
TicketsService::updateTicket() — PATCH /api/v1/tickets/{id} with a company_id, and every other caller of the service;
api/tickets/resolve_triage.php — routing out of triage. The bulk sweep is an UPDATE … WHERE by sender or domain, which does not say which tickets it changed, so the same WHERE is run as a SELECT first to name them.

Why both, and not just the follow-up? Because a follow-up call is a thing every future move path has to remember, and a search filter on the live company is not. The follow-up keeps things tidy; the filter keeps them safe.

4. Workflow tickets had no company

What was wrong

The workflow action Create a ticket (workflow/includes/engine.php) inserted a ticket without a company:

"INSERT INTO tickets (ticket_number, subject, status_id, priority_id,
                     department_id, ticket_type_id, assigned_team_id,
                     assigned_analyst_id,
                     user_id, created_datetime, updated_datetime)

and numbered it as if it had none:

//   No company is stored on this ticket, which means the default one.
$ticketNumber = TicketNumbering::next($conn, $typeId ? (int)$typeId : null, null);

On a multi-company install, no company means triage — so every workflow-raised ticket landed there, whatever ticket fired the rule and whoever it was for. Ticket lists for the Default company include triage tickets (ticketTenantFilter() adds OR tenant_id IS NULL for it), so a workflow ticket raised for company B's requester showed in the Default company's lists. A requester the action created was likewise stored with no company.

The branch just above it — a workflow raising a ticket from a form submission — already set the company from the requester's own record.

The fix

The ticket gets a company, in the same order as that neighbouring branch: the company of the ticket that fired the rule, else the requester's, else triage (multi-company) or the Default company (single-company):

$ticketTenant = null;
$firedFor = (int)(self::dotGet($payload, 'ticket.id') ?? 0);
if ($firedFor > 0) {
    $tt = $conn->prepare('SELECT tenant_id FROM tickets WHERE id = ?');
    $tt->execute([$firedFor]);
    $v = $tt->fetchColumn();
    $ticketTenant = ($v !== false && $v !== null) ? (int)$v : null;
}
if ($ticketTenant === null) $ticketTenant = $userTenant;
if ($ticketTenant === null && !isMultiTenant($conn)) $ticketTenant = getDefaultTenantId($conn);

It is stored in the INSERT and passed to TicketNumbering::next(), so the number comes from that company's numbering. A requester the action creates gets the company their domain maps to, through resolveTenantForNewUser(), as on every other path that creates one.

5. An analyst with no company saw the Default company

What was wrong

getActiveTenantId() in includes/tenancy.php answers "which company is this analyst looking at?", and always answers with some company:

$accessible = getAccessibleTenantIds($conn, $analystId);
return $accessible[0] ?? $defaultId;

For an analyst who may reach no company — a new single sign-on user under "no company until granted", or one whose last company was unticked — that last line returns the Default company. The list filters then filtered to it, so they saw the Default company's tickets, tasks and changes in every list, and its entries in search. The All companies view already failed closed for the same analyst; the single-company view did not.

The fix

Each filter asks whether the analyst can actually reach the company it was given, and if not, shows nothing:

// ticketTenantFilter() and activeTenantFilter()
if (!analystCanAccessTenant($conn, $analystId, $active)) {
    return [' AND 1 = 0', []];
}

Knowledge shows shared articles only (articles with no company, which every company sees), and search shows shared entries only — its company is set to -1, which matches no company:

// knowledgeTenantFilter()
return knowledgeTenantFilterForCompany($conn, analystCanAccessTenant($conn, $analystId, $active) ? $active : null, $alias);

// searchScopeForAnalyst()
if (!analystCanAccessTenant($conn, $analystId, $active)) {
    $scope['tenant_id']       = -1;
    $scope['include_default'] = false;
}

getActiveTenantId() itself is unchanged — a lot of code needs it to return a number. The filters that turn its answer into "what you may see" are what changed.

6. Watchtower counted every company

What was wrong

Watchtower's ticket, change, task and asset cards counted across every company. The code said so, beside the one card that was scoped:

Company scope. NOTE: Knowledge is currently the ONLY card here that scopes by company — the rest of this dashboard (tickets included) is install-wide, which is a pre-existing multi-tenancy gap in Watchtower rather than something this introduced.

So an analyst limited to one company saw the others' volumes — how many tickets were open or unassigned, how many tasks overdue, how many assets were out of warranty or due back off lease. Counts, not records; but counts are information about a company you are not part of.

The fix

New wtCompanyClause() in includes/watchtower_settings.php:

function wtCompanyClause(PDO $conn, int $analystId, string $alias): array
{
    if ($analystId <= 0) return ['', []];
    require_once __DIR__ . '/tenancy.php';
    return ticketTenantFilter($conn, $analystId, $alias);
}

It is the same clause ticket lists use, not a new one — so Watchtower follows the header's company switcher, All companies included, exactly as the lists do, and an analyst with no company sees zeros (section 5). Every ticket, change, task and asset count in includes/watchtower_queries.php adds it, including the ones that were plain query() calls with no parameters and became prepared statements to take one. No analyst (0) means no narrowing, which only an internal caller can be.

7. A failed lookup meant "all companies"

What was wrong

ActorContext — what the services are told about who is acting — carries a company scope, where null means all companies. sessionCompanyScope() in includes/service_context.php works it out for a signed-in analyst, and on any error it returned:

} catch (Throwable $e) {
    return null;
}

So a lock wait or a dropped connection at the wrong moment widened that request's scope to every company.

The fix

It fails closed — an empty list, no companies — except for the one case the rest of tenancy already forgives:

return ($e instanceof Exception && function_exists('tenancyDegradeAllowed') && tenancyDegradeAllowed($e)) ? null : [];

tenancyDegradeAllowed() in includes/tenancy.php allows only a genuinely part-migrated install, where the company tables are not there yet; anything else is logged and denied. The other tenancy guards already went through it after the same mistake was found in them; this one had been missed.

📁 Files changed

	File	Change
🟦	`api/v1/resources/users.php`	§1 — the inferred company is checked
🟦	`api/v1/resources/tickets.php`	§1 — `user_id` and a new requester's company are checked
🟦	`api/tickets/create_ticket.php`	§1 — a new requester's company is checked
🟩	`includes/messaging/ingest.php`	§2 — every channel type threads within its channel
🟨	`includes/search/search.php`	§3 — ticket entries judged by the live company; §5 — no company, shared only
🟨	`includes/tenancy.php`	§3 — `ticketsFollowCompanyMove()`; §5 — the three filters
🟨	`api/tickets/move_ticket_to_company.php`, `api/tickets/resolve_triage.php`, `includes/services/tickets.php`	§3 — call `ticketsFollowCompanyMove()`
🟧	`workflow/includes/engine.php`	§4 — the ticket and a new requester get a company
🟪	`includes/watchtower_settings.php`, `includes/watchtower_queries.php`	§6 — `wtCompanyClause()`, on every count
🟪	`includes/service_context.php`	§7 — fails closed
⬜	`tests/security-report-2026-10.php`, `tests/security-report-2026-10-part2.php`	the checks below

Tested

Two test files, against the real database and web server, with temporary data they remove afterwards. Most of these checks need two companies, and are skipped, with a note, on a single-company install.

§1 — requesters. tests/security-report-2026-10.php section 5 covers POST /users (see four smaller gaps). tests/security-report-2026-10-part2.php section 4b, with a temporary key limited to one company, a temporary requester and a temporary email domain in the other:

naming the other company's requester by id is refused with 422 and "no longer exists";
an address on the other company's domain is refused with 403, and no requester was created;
POSITIVE CONTROL: the same key, sending no subject, gets the endpoint's own 422 for the subject — so the refusals above came from the new checks, not from a key that never reached the endpoint;
and no ticket was created by any of it. Each request is built to be refused, because a ticket that was actually created would fire the install's real ticket.created workflows.
The New ticket form's check is confirmed in the source: create_ticket.php calls resolveTenantForNewUser() on the typed address.

§2 — WhatsApp. tests/security-report-2026-10.php section 7: the open ticket is found from the same channel and not from another.

§3 — moves. Part 2, section 12, with a temporary ticket, a message, a task and a subtask in one company:

POSITIVE CONTROL: that company finds the ticket by a word in its message;
after a raw UPDATE of the ticket's company — the shape of a path that forgets the follow-up — the old company no longer finds it. This is the live-company filter working on its own;
after ticketsFollowCompanyMove(), the new company finds it, and the task and the task's subtask are both in the new company;
the source of the Move button, TicketsService and triage routing all call it.

§4 — workflow. Part 2, section 13: the source of the Create a ticket action stores tenant_id and passes the company to TicketNumbering::next(). Not run live — creating a ticket would fire real workflows.

§5 — no company. Part 2, section 23, with a temporary analyst given no company: they can reach no company (setup); ticket lists and every other company-scoped list return AND 1 = 0; Knowledge returns shared articles only; search's company is -1. POSITIVE CONTROL: an all-companies analyst is not filtered to nothing.

§6 — Watchtower. Part 2, section 24: wtCompanyClause() returns exactly what ticketTenantFilter() returns for the same analyst, and watchtower_queries.php uses it at least six times.

§7 — failed lookup. Part 2, section 24: the source of sessionCompanyScope() returns [] unless tenancyDegradeAllowed() says otherwise. A database error cannot be caused on demand in a test against a real install, so this is checked in the code, not provoked.

Do I need to do anything?

Upgrade. Nothing to configure. Three things you may notice afterwards:

Workflow tickets now land in a company instead of triage. If you have been routing them out of triage by hand, you will not need to.
An analyst with no company now sees empty lists instead of the Default company's. If somebody tells you their queue went empty after the upgrade, give them a company under their analyst record — that is what they were missing all along.
REST integrations that raise tickets with a company-limited key and name a requester from another company, or create one on another company's domain, now get 422 or 403. That was never meant to work.

If you want to tidy up after §3, tickets moved between companies before the upgrade may still have tasks in the old company, which can be moved by hand from Tasks. The old company stops finding those tickets in search as soon as you upgrade; the new company finds each one once it is next re-indexed, which a reply on the ticket does.

For developers

"The caller must have authorised this" is a contract, and every caller signs it. When a service takes an id on trust, read its comment, then check every adapter that calls it — the UI one and the REST one are written at different times, by different hands.
Inferred values need the same check as supplied ones. A company that comes from an email domain, a channel, a parent record — wherever it comes from, ask whether the caller may reach it.
When the same rule has been added case by case, replace the list with the rule. Telegram, Mattermost, Teams, then WhatsApp was missed. $channelId !== null covers the next one.
Prefer a check at the read over a follow-up at every write. A filter on t.tenant_id in the one search query protects every move path, including ones not written yet. A follow-up call protects only the paths that remember it. Do both, but know which one is the safety.
A fallback that picks "some company" must not pick one the analyst cannot reach. If you turn getActiveTenantId() into a filter, check analystCanAccessTenant() on the answer first — or use ticketTenantFilter() / activeTenantFilter(), which now do.
In tenancy, null means "everything". Never return it from a catch. Fail closed, through tenancyDegradeAllowed().
A comment admitting a gap is a bug report. Watchtower's was accurate, and it sat there.