Commit Graph
13888 Commits
Author SHA1 Message Date
nicandClaude Opus 5.5 25dba32671 fix: restart zmcontrol.pl when a monitor is edited through the API (#5205)
MonitorsController::edit() restarted zmc through the CakePHP model but
left the monitor's control server running. zmcontrol.pl reads
ControlAddress, ControlDevice and the Control when it starts, so after
an API edit PTZ kept using the old values: a camera password changed
through the API kept failing PTZ with 401 until zmcontrol.pl was
restarted by hand.

Restart the daemons the way the console's save does
(includes/actions/monitor.php), through ZM\Monitor: stop zmc and, for a
controllable monitor, zmcontrol.pl; start them again from the saved row
unless the monitor does not capture, is a WebSite or is deleted.
ZM\Monitor also sends the commands to the monitor's own server in a
multi-server setup, where the model's daemonControl() logged an error and
did nothing.

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-11 11:23:46 -04:00
Isaac ConnorandClaude Opus 5.5 13138a0afc fix: check each event's monitor when editing event details
actions/eventdetail.php checked only the global Events=Edit permission and
then updated the Name, Cause and Notes of every id in markEids, so a user
denied some monitors could rewrite those monitors' events. The event detail
modal likewise showed the name, cause and notes of any event id it was given.

Load each event and require $event->canEdit(), which adds the per-monitor
view check, as archiving and unarchiving in actions/events.php already do.
The action skips events that fail it; the modal leaves them out, and shows
nothing for a single event that fails.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-11 11:21:55 -04:00
Isaac ConnorandClaude Opus 5.5 7eb7dc4bff fix: only redirect back to the Referer from control and event detail actions when it is on this server
actions/control.php and actions/eventdetail.php redirected to
$_SERVER['HTTP_REFERER'] as is. That is the Referer of the POST itself, which
normally comes from a ZoneMinder page, and the CSRF token keeps other sites
from making the POST, so it isn't the open redirect the zone save had. But it
is the same pattern, and a missing Referer warned about an undefined index and
rendered the action's view instead of redirecting.

Use localRedirectUrl() as the zone save does, falling back to the monitor's
control view and to the events list.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-11 11:19:01 -04:00
Isaac ConnorandClaude Opus 5.5 16817e4ffc fix: only redirect back to a zone's Referer when it is on this server
After saving a zone, actions/zone.php redirected to the REFERER the zone form
posted, which the form fills from the Referer its page was loaded with. A
zone editor who reached the zone page through a link on another site was sent
to that site after saving: an open redirect, usable to lead someone from a
trusted ZoneMinder page to a look-alike login. Forging the POST itself is
blocked by the CSRF token, so it needs an editor who followed such a link.

localRedirectUrl() returns the target only if it is a query-only URL or an
absolute http(s) URL on the host and port of the current request, and the
fallback otherwise: another host, //host, backslashes, user@host, other
schemes and control characters all give the fallback. The zone save falls
back to the zones list, as it already does with no Referer (#5207).

tests/php/test_local_redirect_url.php covers 21 cases.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-11 11:16:14 -04:00
nicandClaude Opus 5.5 6a35e89c14 fix: go to the zones list after saving a zone opened without a Referer (#5207)
The zone form posts its Referer back as REFERER, and posts it empty when
the form was opened from a typed URL or a bookmark. The action used it
whenever it was set, so $redirect was empty, index.php rendered the zone
view with the action's $zone (a database row, or array() for a new zone,
neither with Points) and ksort() at views/zone.php:105 failed with
'Argument #1 must be of type array, null given'. The zone was saved but
the browser got a 500.

Use REFERER only when it is not empty and fall back to the monitor's
zones list.

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-11 11:15:08 -04:00
nicandClaude Opus 5.5 f4b46657b3 fix: stop fixAmps() recursing forever on a navbar link with an entity (#5210)
* fix: stop fixAmps() recursing forever on a navbar link with an entity

fixAmps() is declared with one parameter but called itself with a
position, which PHP drops, so every call searched from the first '&'
again. Once ZM_WEB_NAVBAR_LINKS held an entity (&amp;, &#39;, ...) or a
bare '&' followed later by a ';', rendering the left navbar recursed until
PHP ran out of memory, and every page answered 500 for users who use the
left navbar.

Escape the bare ampersands in one preg_replace() instead. Character
references are matched at the '&' itself, so a bare '&' is no longer
taken as valid because an entity appears later in the string, and
hexadecimal references (&#x27;) are kept.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix: read the view of a navbar link only when its href has one

getAdditionalLinksHTML() parsed every left navbar link's href as if it
carried a query string with a view parameter. A link to another site
(https://host/docs) logged 'Undefined array key "query"', a parse_str()
deprecation and 'Undefined array key "view"' on every page view.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-11 11:04:47 -04:00
Isaac ConnorandClaude Opus 5.5 53662582f0 fix: stop the event view resizing and wiping on every gapless event
Gapless playback loads a new event page for each event, and each load moved
the video three times and ended in a page-wide wipe:

- video.js sets the player up on DOMContentLoaded without a size, while the
  first changeScale() ran from initPage() on window load. The browser could
  paint a collapsed video with the controls drawn up under it, and
  changeScale() measured that collapsed layout (an empty inline-block still
  leaves a line box), which made the video too small by about 17px.
- skin.js's ResizeObserver on the navbar statistics called endOfResize() for
  the size it reports when it starts observing, and for every width change of
  the figures, so changeScale() ran again 250ms later and corrected the size.
- With the stats panel turned off, initPage() hid it and widened the video's
  column only after load.
- The page-leave roll-up in skin.js animated the content to nothing on every
  reload, a wipe from the bottom up between events.

Size the video on DOMContentLoaded, after video.js has the player, and
remember the size per monitor (zmEventVideoSize<Id>) so event.php opens the
next page at it; changeScale() still corrects it if the window or layout
changed. Render the stats panel and the video's column width from the
zmEventStats cookie in event.php. Rescale for the navbar only when its height
changes. Move between events through goToEvent(), which sets
skipPageLeaveAnimation so the roll-up is skipped for that reload.

Measured in Chrome at 1920x915 on a 1280x720 event, each frame's video size
and controls position: before, every page went 1x0, 1144x644, 1174x660 with
the controls starting near the top of the video, and the content shrank from
851px to 363px before the reload. After, the page reached by gapless playback
paints at 1174x660 with the controls in place and does not move, and there is
no roll-up. The first event of a session still sizes once, as no size has
been remembered yet. Window resize and the stats button still rescale, for
both video.js and zms events.

tests/js/event-gapless-layout.test.js covers goToEvent() and the
DOMContentLoaded sizing; two of its three tests fail without this change.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-11 10:28:01 -04:00
Isaac ConnorandClaude Opus 5.5 821030e17e fix: read API request fields the way Model::set() saves them refs GHSA-fp33-8fx5-4j6c
requestField() read Model[field] whenever the request's Model entry was an
array, empty or not. CakePHP's Model::set() treats the bare top-level fields
as the model's data when that entry is empty (_setAliasData), so
{"Monitor":[],"EventEndCommand":"x"} passed the System-edit check on the
monitor command fields, which found no value, while save() stored x. A
Monitors=Edit user without System=Edit could set EventStartCommand or
EventEndCommand, which zmc runs through /bin/sh.

Every other check built on requestField() had the same gap: the target
MonitorId when adding or editing an event, the EventId when adding a frame,
and the ParentId when editing a group.

Read Model[field] only when the Model entry is non-empty, as set() does, and
otherwise the bare field; a non-empty entry that is not an array yields
nothing, since set() skips it.

tests/php/test_api_request_field.php runs the real method against each
request shape.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-10 13:36:01 -04:00
Isaac ConnorandClaude Opus 5 6c0f976c22 refactor: move the presentation settings to the Display tab
The skin and css, the language and locale, the date, time and datetime
formats, the default bandwidth and the snapshots feature switch all
decide how the interface looks or what it offers, which is what that tab
is for, but they sat under System among the shutdown command, the shm
key and the audit intervals. System drops from 42 entries to 33 and
Display goes from 32 to 41.

The snapshots switch is read only by the skin: it gates a button in
montage and montage review, a menu item, and the snapshot views and
their permission checkboxes. Nothing in the daemons or the perl scripts
looks at it.

The category key is still 'web'. That is the tab now labelled Display;
the tab whose key is 'display' holds the per-session cookie settings and
stores no configuration at all, so a stored default does not belong
there. The hint setup for the moved entries -- the installed skin list,
the css list for the current skin, the bandwidth options, and the
language and locale lists -- moves to the same branch.

ZM_TIMEZONE stays under System. It reads like a presentation setting,
but Filter.pm uses it for date arithmetic when evaluating filters, so it
changes behaviour and not just what is displayed.

A migration, 1.39.40, is needed as well as the ConfigData change, because a
version upgrade does not rewrite Config: zmupdate only reloads it from
ConfigData under --freshen, which is mutually exclusive with an upgrade
run. Fresh installs take the new category from the generated
zm_create.sql.

Verified both paths converge on 33 system and 41 display entries with
identical membership: a fresh install of this branch, and a copy of a
real pre-change Config table with the migration applied. On the running
install the bandwidth, language and locale entries still render as their
pickers on the new tab, css as its list of choices, and the timezone
picker is still on System with its full list.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Y6FieTwEXuLhhR4e2yiax
2026-10-10 09:32:17 -04:00
Isaac ConnorandClaude Opus 5 7f7c9cc81b refactor: name the options tabs after what they hold
The Display tab holds skin, css and navbar choices kept in a cookie for
the session looking at the page. They are not stored configuration and
do not affect anyone else, but the tab was named as though they were the
system's display settings. The settings that genuinely are the system's
display configuration -- the navbar, titles, thumbnails, event sorting,
animations, alarm popups -- sit on a tab labelled Web, which says where
they apply rather than what they do.

So the labels swap to match: the cookie tab becomes This Session, and
the Web tab becomes Display. A short note at the top of This Session
says the settings are kept in a cookie rather than saved on the server,
since a name alone will not carry that. The note goes inside the form
rather than beside it, because #optionsContainer is a flex row and two
siblings there split the width between them.

Labels only. The category keys are untouched, so ?tab=display and
?tab=web keep working, no configuration entry moves, and no migration is
needed. The one-off version/Versions case that was inline in the menu
builder becomes a small map beside the other two, applied at both places
that build the item list.

Only en_gb gains the new strings. Language files are loaded with en_gb
as the base and the chosen language layered over it, so the other 27
show the English until somebody translates them. Display and Web are
already translated in 22 of them, and those translations stay correct:
the word Display now labels the tab that actually holds display
settings.

Verified on a running install: the sidebar renders This Session for
tab=display and Display for tab=web, the note spans the full width above
the fields, all three tabs return 200 with their entry counts unchanged
and no php diagnostics.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Y6FieTwEXuLhhR4e2yiax
2026-10-10 09:32:17 -04:00
Isaac ConnorandClaude Opus 5 1043273495 refactor: drop the unused Type column from Controls
Controls.Type looks like the monitor type a control applies to, and has
been kept in step with the Monitors enum since 1.27, but nothing has
ever read it. The control dropdown on the monitor page lists every row
by Name, no query filters on it, and no code compares it to the
monitor's own Type. It was a field the edit form asked for and the
control list displayed back.

The column appears to be MonitorPresets.Type copied by analogy, where
the idea is real: that one decides what a preset creates. That also
explains why every migration altering one altered the other.

Removes the column from the schema, adds the 1.39.39 migration, the edit form row
and its type list, the control list column, the defaults in Control.php
and Control.pm, and the value from all 53 shipped rows in controls.sql.
This supersedes the 1.39.38 retyping of three libvlc controls to Ffmpeg:
the column they were being retyped into is gone.

zmcamtool needs no change -- its import regex rewrites the Id and
captures the Name, so it is not positional past that, and its export is
a mysqldump that follows the live schema.

Dropping Type left the two shipped Pelco rows byte-identical to their
twins, which existed only as a Local and an Ffmpeg copy of the same
control, so the redundant copies are removed: 53 shipped rows become
51, with both Pelco-D and Pelco-P still present once.

Existing installs are deliberately left with their duplicates. The same
pair collapses there too, but deleting rows a monitor's ControlId may
point at, to tidy a repeated name in a dropdown, is not a trade a
migration should make silently.

Verified by loading the old and rewritten controls.sql into scratch
databases and diffing every remaining column across all 53 rows: no
value shifted, which is the failure mode when a positional INSERT loses
a field. The file mixes positional and explicit-column INSERTs, so both
forms were handled. Fresh-install and migrated Controls schemas compare
equal, the shipped data loads into both, and Monitors and MonitorPresets
keep their Type. Build clean, 147 test cases pass, php -l and perl -c
clean on the edited files.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Y6FieTwEXuLhhR4e2yiax
2026-10-10 09:32:17 -04:00
Isaac ConnorandClaude Opus 5 bf8a73e9fb feat: remove Libvlc from the web ui, api and config help
Follows the daemons: no monitor can be type Libvlc any more, so the ui
should not offer it or carry text describing it.

Drops the source-type option and its deprecation notice, the Libvlc arm
of the Source Path and console source-column branches, the entry in the
control-capability type list, and the value from the api's Type
validation list. The OPTIONS_LIBVLC help block goes from all 25 language
files: monitor.php builds that key as OPTIONS_ plus the monitor type, so
it became unreachable the moment the type did. The Libvlc source-type
label goes from the 22 files that carried it.

SourceTypeDeprecated and Deprecated lose their last caller and are
removed. RemoteRtspDeprecated and Removed stay -- the rtsp protocol
option is still shown to monitors already set to it.

The WEB_FILTER_SOURCE help listed the affected source types; corrected
in ConfigData.pm.in and in the one translation that had localised it.

All 25 language files and the four edited php files parse clean under
php -l, and the config module parses under perl -c. No javascript
changed, so eslint has nothing to say here -- it cannot run in this
worktree anyway, which has no node_modules.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Y6FieTwEXuLhhR4e2yiax
2026-10-10 09:32:17 -04:00
Isaac ConnorandClaude Opus 5 fd0450c160 feat: remove the NVSocket capture method
NVSocket spoke a bespoke socket protocol to one vendor's nvr. It has a
single known user, no way for anyone else to test it, and nothing it
reaches that ffmpeg cannot reach over rtsp or http, so it is removed
rather than carried further.

Drops RemoteCameraNVSocket, the NVSOCKET camera type and its parse and
construction arms, the source-type option and settings panel in the web
ui, and the stray ja_jp translation key. The preceding migration moves
any existing rows off the type first, so Monitor::Load cannot meet a
type it no longer recognises -- that path is Fatal and would stop zmc
rather than skip the monitor.

Built clean; unit suite passes (147 cases, 12531 assertions, run from
tests/ where the font fixtures' relative paths resolve). eslint clean.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Y6FieTwEXuLhhR4e2yiax
2026-10-10 09:32:17 -04:00
Isaac ConnorandClaude Opus 5 ce16d3eb08 feat: deprecate the Libvlc and NVSocket capture methods
Both reach cameras that ffmpeg reaches. Libvlc does it through a second
streaming library with its own build dependency, threading and bugs; NVSocket
speaks a bespoke socket protocol for one vendor's nvr that we have no way to
test. Neither earns the maintenance, and the same monitors work as Type Ffmpeg
with the camera's rtsp or http url.

Deprecated, not removed. A monitor using either still captures; it logs a
warning at startup naming what to change it to, and the form marks the type and
explains. That follows what Remote/RTSP did before it was removed, which is the
shape that gives people a release to move in.

The warnings were verified firing rather than assumed: switching a monitor to
each type in turn produces the expected message naming that monitor.

php -l and eslint clean; the three pre-existing duplicate lang keys are
unchanged. Suite 147 cases / 12531 assertions.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Y6FieTwEXuLhhR4e2yiax
2026-10-10 09:32:17 -04:00
Isaac ConnorandClaude Opus 5 af0d6ddc32 refactor: remove RTSPDescribe, which lost its reader with the rtsp client
RTSPDescribe chose whether to take the media url from the DESCRIBE
response rather than the configured path. That was a quirk of
ZoneMinder's own rtsp implementation; ffmpeg decides it itself. Removing
that client left the setting with nothing behind it -- the monitor form
still drew the checkbox and the save path still wrote the column, but no
code read it, so the only thing it could do was mislead whoever ticked
it.

Drops the form row and the Remote-only block that existed to hold it,
the javascript that showed and hid it as the protocol changed, the field
in the php and perl monitor models, the default in the save action, the
column from the schema, and the label and help text from all the
language files carrying them.

The 1.39.37 migration drops the column behind an information_schema
guard scoped to DATABASE(), so the file stays re-runnable.

Verified by building a pre-upgrade database from upstream/master's own
schema and applying the migration: the column goes, and Monitors matches a fresh install of this
branch column for column. Migration applied three times with no error.
php -l clean on the four edited files and all 28 language files, perl -c
clean, build clean, 154 test cases pass.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Y6FieTwEXuLhhR4e2yiax
2026-10-10 09:32:17 -04:00
Isaac ConnorandClaude Opus 5 a88c6859ca fix: stop the web offering an rtsp protocol that no longer captures
With the rtsp client gone, Type=Remote with Protocol=rtsp cannot capture, so
the monitor form no longer offers it and the camera probes no longer suggest
it.

A monitor already saved as rtsp still sees the option, because dropping it
outright would mean opening such a monitor to read the warning silently
re-saved it as http -- swapping one broken configuration for another. For those
the option is labelled removed and the warning now says the monitor will not
capture until it is changed, rather than that it will stop working in a future
release.

The three probes that suggested Remote/rtsp -- ACTI in both probe
implementations, and Vivotek -- now suggest Type Ffmpeg with the equivalent
rtsp:// Source Path, which is the same stream by a maintained route.

Type=Remote keeps http. mjpeg and snapshot-jpeg cameras are what it is for.

php -l clean on all four files; eslint clean; the three pre-existing duplicate
lang keys are unchanged.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Y6FieTwEXuLhhR4e2yiax
2026-10-10 09:32:17 -04:00
Isaac Connor f449b92a3d Only show options sidebar if it isn't in the left menu 2026-10-10 09:06:44 -04:00
Isaac Connor 793684d6b9 SIDEBAR_FOOTER is not part of the actual menu 2026-10-10 09:06:44 -04:00
Isaac ConnorandClaude Opus 5.5 0e9f82513c fix: bind the FOSCAMR2C preset and require Control permission for PTZ refs GHSA-qcm7-vq92-f86f
FOSCAMR2C presetGoto and presetSet interpolated the preset into
"SELECT Label FROM ControlPresets WHERE Preset = $preset" and sent the selected
Label to the camera as name=. For presetGoto the preset came straight from the
web request, so any user who could see the monitor could run a SELECT against
the ZoneMinder database and have the value (the auth hash secret, password
hashes) delivered to the camera link, or use it as a boolean oracle.

- FOSCAMR2C: bind the preset in every statement, refuse a preset that is not a
  number, and look ControlPresets up by MonitorId as Dahua and PSIA do.
- buildControlCommand(): refuse a non-numeric presetGoto preset, and a control
  name that is not a plain method name. zmcontrol calls the name as a method and
  the options are split back out of the command string on spaces.
- zmcontrol.pl: refuse a command that is not a plain method name, so a qualified
  Package::sub name can't call a sub outside the control module.
- canView()/canEdit() with a monitor id returned only the monitor check and
  ignored the area, so canView('Control', $mid) passed for anyone who could see
  the monitor. Require the area permission as well; Monitors stays decided by
  visibleMonitor()/editableMonitor().

tests/php/test_control_command.php covers buildControlCommand() and the
monitor-scoped canView()/canEdit(); tests/perl/test_foscamr2c_preset_sql.pl
checks the statements FOSCAMR2C prepares and what it sends to the camera.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-08 18:21:32 -04:00
Isaac ConnorandClaude Opus 5.5 1b2bc60936 fix: give ZM\User real canView/canEdit/canCreate area checks refs GHSA-ff93-w6fm-vqxr
Filter::canView(), canEdit() and canDelete() ask the user object
$u->canEdit('System'), $u->canView('Events') and so on, but ZM\User had no such
methods. The calls fell through to ZM_Object::__call(), which stored the argument
as a property and returned it, so every check returned a truthy area name. Any
account with Events=View passed as a System editor and could save and run a
filter with AutoExecute, an OS command run by zmfilter.pl. The same fall-through
made the filter ownership and filterdebug checks pass for everyone.

Add canView(), canEdit() and canCreate() to ZM\User. They take the level from the
user's own field and from their role, the way the global functions in auth.php do
for the logged-in user, and refuse anything that is not a permission area.
Monitor::commandFieldChangesNotAllowed() now uses canEdit('System') so a role's
System=Edit counts there too.

In the filter action, require edit on a stored filter before applying the
submitted fields when saving over it, since the fields can name a new owner;
stop after a refused save instead of starting the filter in the background; and
require edit on the filter for start/stop/restart, which had no check.

tests/php/test_user_permissions.php drives the real ZM\User, ZM\User_Role and
ZM\Filter classes. The older filter tests stubbed the user with its own
canView/canEdit, which is why they passed while the real class had neither.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-08 18:14:17 -04:00
Isaac ConnorandClaude Opus 5.5 c8eb03b4f2 fix: encode values written into page scripts, montage review dates and colours refs GHSA-qm44-862r-fmq3 GHSA-8xqq-932v-9h3j
Montage review wrote two values into single-quoted JavaScript strings
unencoded, inside the page's nonce-bearing script:

- minTime/maxTime, which can come from the date terms of a filter in
  the request, so a crafted link ran script for the Events viewer who
  opened it (GHSA-qm44-862r-fmq3). Stored cookies fed them the same way.
- Monitor.WebColour, which any editor of that monitor can set through
  the API, so it ran script for whoever viewed montage review
  (GHSA-8xqq-932v-9h3j).

Keep only range values that parse as a time, in canonical form, and
write minTime, maxTime and monitorColour with json_encode(). The canvas
style attribute took WebColour raw as well; escape it for HTML.

The same unencoded pattern ran through the other page-script templates,
where earlier fixes had covered named fields one at a time. Pass every
value echoed directly into a quoted string in web/skins/classic/views/js
through validJsStr(), 68 in all, including the timeline view's own
minTime/maxTime and request-derived values such as rate, scale, mode and
streamMode. Numeric and date values are unchanged by it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-08 08:20:45 -04:00
Simpler1 c9338125b9 Keep preview from bleeding through the tags dropdown (#5193)
Removed z-index property from #frames img.

I'm not sure if this will have any other adverse effects, but I haven't found any.  

When there are many tags in the dropdown list, the previews bleed through the list and make the tags unselectable where the previews are.  This change allows the tags dropdown to display over the previews as they should.
2026-10-07 18:45:18 -04:00
Isaac ConnorandClaude Opus 5.5 1677470af1 fix: require System edit to change a monitor's event commands refs GHSA-vcrj-35c6-g97c
zmc runs a monitor's EventStartCommand and EventEndCommand at the start
and end of each event. Both the classic save and bulk edit actions and
the API monitor add and edit authorized only on the monitor, so a user
with edit on one monitor and no System permission could set a command
for zmc to run as the ZoneMinder service user.

Changing either field now needs System=Edit, as Filter::canEdit()
already requires for AutoExecuteCmd. Monitor::commandFieldChangesNotAllowed()
reports the command fields a change would alter; submitting the current
value is not a change, so other users can still save the rest of the
monitor. The monitor form shows the fields read-only to them. Also
close the EventEndCommand list item.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-07 08:09:33 -04:00
Isaac ConnorandClaude Opus 5.5 47ec378403 fix: do not show Private option values to System viewers refs GHSA-7g3f-93q8-fhwj
The Options page admits System=View and wrote every Config value into
its inputs, only disabling them for users who cannot edit. The Private
flag was never consulted, so a read-only administrator received
ZM_AUTH_HASH_SECRET, which is enough to sign a token for any user, and
the other Private credentials on their tabs.

For a Private option, show its value only to System editors; others see
that it is hidden. The note about a value overridden in zm.conf, which
printed the overriding value, is skipped for those options too.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-07 08:09:33 -04:00
Isaac ConnorandClaude Opus 5.5 22f9413333 fix: keep zm.conf values out of the browser refs GHSA-jv9c-pg2m-5m6m
Since b2861d5cf loadConfig() returns the Config table merged with the
zm.conf and conf.d values that have no Config row. Those carry no
Private flag, and skin.js.php writes every config entry not marked
Private into each page as a JavaScript const, for any logged-in user
whatever their permissions. ZM_DB_USER and ZM_DB_PASS were in every
page, the permission error page included. The API's Configs listing
redacts by the same flag.

Mark values that exist only in the config files Private, apart from
ZM_DIR_SOUNDS, the one the browser uses (for the alarm sound).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-07 08:09:33 -04:00
ad33e3007e Fix: JPEG snapshot generation when requested frame is unavailable in event video (#5184)
* Fix: JPEG snapshot generation when requested frame is unavailable in event video

* Improve frame extraction fallback for unavailable timestamps

- Limit ffprobe search to a small time range around the requested position
- Validate the generated JPEG before considering extraction successful
- Fix timestamp formatting to be locale-independent
- Remove obsolete command/output handling from callers
-Handle non-monotonic packet PTS order
-- Do not assume ffprobe returns packets sorted by PTS
-- Scan all packets in the selected time range
-- Select the closest packet before and the closest packet after the requested timestamp
-- Avoid selecting an incorrect fallback frame when PTS values are reordered

* Use temporary files for frame extraction

- Write FFmpeg output to a unique temporary file for each extraction attempt
- Atomically rename the validated frame to the target path on success
- Prevent failed concurrent requests from deleting a frame created by another request

* Fix video frame extraction near event end

* Remove frame_time condition from lookback loop

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

---------

Co-authored-by: Isaac Connor <isaac@zoneminder.com>
Co-authored-by: Isaac Connor <iconnor@connortechnology.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
2026-10-06 17:41:30 -04:00
IgorA100 c6b77d44d7 Preserve non-ASCII characters in log messages (#5188) 2026-10-06 08:56:35 -04:00
Isaac ConnorandClaude Opus 5.5 575e5484b3 fix: skip an unconfigured go2rtc under the Auto player
With its player left on Auto, a monitor with go2rtc enabled had
selectNextPlayer() pick go2rtc_webrtc first, and with ZM_GO2RTC_PATH
empty select_go2rtc() then alerted "ZM_GO2RTC_PATH is empty", on the
montage for every such monitor, although nobody had chosen go2rtc.

The fallback chain already skipped go2rtc for a monitor without it
enabled; skip it when ZM_GO2RTC_PATH is empty too. A monitor or browser
that chose go2rtc reaches select_go2rtc() directly and still gets the
alert.

A node test drives selectPlayer() through the restart chain for Auto
without a path, Auto with one, and go2rtc chosen explicitly.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-05 18:36:33 -04:00
Isaac ConnorandClaude Opus 5.5 c3ff83fa56 fix: do not use the RTSP restream when ZM_MIN_RTSP_PORT is not set
ZoneMinder's RTSP server only starts when ZM_MIN_RTSP_PORT is set, and it
defaults to empty. The go2rtc and Janus managers decided to use the
restream from the monitor's RTSP Server flag (Janus: its restream flag)
alone, so with the port unset they built rtsp://127.0.0.1:0/<name>.
go2rtc was given that as the monitor's primary stream and as
<id>_ZoneMinderPrimary, neither of which can play; Janus pointed its
mountpoint at it.

Use the restream only when the port is set as well, and warn when a
monitor asks for it without one, so the camera's own stream is used.
MonitorStream applies the same test before choosing the
_ZoneMinderPrimary stream, which is no longer registered in that case.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-05 18:34:04 -04:00
Isaac ConnorandClaude Opus 5.5 0d6ebcd42c fix: accept ZM_GO2RTC_PATH with or without its /api suffix
go2rtc serves its API under /api, and its websocket at /api/ws. zmc
appends /streams to ZM_GO2RTC_PATH and the browser appends /ws, so the
setting only worked if it ended in /api. The natural value to enter is
the server address, http://host:1984, and with that zmc reached
/streams, which is go2rtc's RTSPtoWeb compatibility endpoint, so no
stream was ever registered, and the browser's /ws was a 404. Where
another player was available the failure went unnoticed.

Normalise the setting where URLs are built from it: a trailing slash is
dropped and /api is added unless the path already ends in it, which also
covers go2rtc behind a reverse proxy at a sub-path.

- Go2RTCApiEndpoint() in zm_utils, used by zmc's Go2RTCManager.
- go2rtcApiUrl() in skin.js, used by MonitorStream and the console
  thumbnail stream. It assigns the path once, since stripping the slash
  from a bare host's "/" and appending gave "//api", as the old
  pathname += '/ws' gave "//ws".
- The option's help says the suffix is optional.

Catch2 and node tests cover bare hosts, trailing slashes, an existing
/api, a proxy sub-path and credentials in the URL.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-05 18:34:04 -04:00
Isaac ConnorandClaude Opus 5 a5860e02b2 fix: read MJPEG frames with createImageBitmap so Firefox keeps up
EventStream receives the stream into a DOM <img> and copies it to the
canvas from a rAF loop.  Gecko hands canvas drawImage() the frame the
img held when it was first decoded and never refreshes that snapshot,
so a multipart/x-mixed-replace img drawn to a canvas repaints one frame
forever while the stream keeps arriving.  Montage review showed a single
frozen frame in Firefox while the timeline advanced; Chrome, whose
drawImage() tracks the element, played the same streams normally.

createImageBitmap() reads the frame the element is showing now.  It is
async, so a tick is skipped while a request is outstanding rather than
queueing requests up behind the stream rate, and the pending flag is
cleared on teardown along with the img.

The img is left offscreen: measured against a reference
multipart/x-mixed-replace server, an img at left:-9999px, 1x1,
visibility:hidden still yields a fresh frame on every
createImageBitmap(), so Gecko is not culling the decode.

Verified against montage review on Firefox 154 and Chrome 154, driving
both from a script that hashes each monitor's canvas 40 times at 250ms:
before, Firefox saw 1 distinct frame per canvas and Chrome 40; after,
both see 40.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KLYswrguKc6bBYL8JZbpmZ
2026-10-05 17:24:13 -04:00
Isaac ConnorandClaude Opus 5.5 222b4d7c9e fix: stop the Intel GPU view hanging under a threaded Apache MPM fixes #5166
Under a threaded MPM (event/worker) Apache blocks signals in its worker
threads and children started by exec() inherit that mask. Where /bin/sh
is bash (Gentoo, Fedora/RHEL, Arch) the mask reaches intel_gpu_top, which
never sees the SIGTERM from "timeout 2", so exec() never returned, the
page never loaded and every visit left another intel_gpu_top running.
dash, Debian's /bin/sh, clears the mask, which is why it doesn't show
there.

- Run the sample with -n 2 where intel_gpu_top supports it, so it exits
  by itself after one full period. Support is detected from -h, because
  intel-gpu-tools 1.26/1.27 (Ubuntu 22.04, Debian 12) lack -n.
- Without -n, keep "timeout 2" but add -k 3, so a blocked SIGTERM is
  followed by an unblockable SIGKILL. The samples printed before the kill
  are still parsed; only when there are none does the view explain the
  kill instead of showing a raw-output error.
- Give the -h and -L calls a SIGKILL timeout too.
- Report the driver bound to the Intel DRM device and the kernel version
  instead of /sys/module/i915/version or modinfo, neither of which gives
  a version for i915 or xe, and neither of which exists when the driver
  is built in, so Driver always showed Unknown.
- Add dark theme styles for the view's cards, card headers, footer row
  and progress bars, which kept Bootstrap's light backgrounds behind the
  dark theme's light text.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-04 12:29:13 -04:00
Isaac ConnorandClaude Opus 5 8694032b49 fix: validate Range requests in view_video instead of trusting them refs #5174
view_video.php took the end of the range straight from the request and never
checked it against the file:

  if (!empty($matches[2])) $end = intval($matches[2]);
  $length = $end - $begin + 1;

so on a 1000 byte file:

  bytes=0-99999    206, Content-Length 100000, body 1000 bytes
  bytes=5000-6000  206, Content-Range naming bytes that do not exist
  bytes=500-100    206, Content-Length -399
  bytes=1000-      206, Content-Length 0
  bytes=-100       200 with the whole file, not the last 100 bytes

A client that is told to expect 100000 bytes and gets 1000 does not see a bad
request, it sees a truncated file, and reports the video as broken. The suffix
form was not recognised at all because the pattern required a digit before the
dash.

This is reached once per fragment by the byte-range HLS manifest VideoStore
writes, every fragment being a Range against the one mp4, so a player that
asks for anything the file cannot supply gets a body that does not match its
own Content-Length rather than an answer it can act on.

Parse the header properly: clamp a range that runs past the end, because a
client may ask for more than is there and is entitled to what is there;
answer 416 with "Content-Range: bytes */size" when the range cannot be
satisfied at all, so the client learns the real length; and read "-N" as the
last N bytes. Length is now derived from the range being served rather than
the one requested, and the send loop counts down by the bytes it actually
read, so Content-Length and the body cannot disagree.

Only the first range of a multi-range request is served, as before. A
multipart/byteranges body is not worth building for this, and falling back to
sending the whole representation is not an option when these are event videos
of hundreds of megabytes; Content-Range names exactly what was sent.

The parsing is its own dependency-free include so it can be tested without a
database, and tests/php/test_http_range.php covers each case above plus a
sweep asserting that every range it ever returns lies inside the file.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-10-04 08:26:42 -05:00
nicandClaude Opus 5.5 2539e6a157 fix: resolve ZM_SERVER_NAME to its server id in config.php (#5171)
8c35190b0 moved the Servers lookup into a foreach over $s but kept
define('ZM_SERVER_ID', $Server->Id()), so every request on a node with
ZM_SERVER_NAME in zm.conf died with 'Call to a member function Id() on
null'. Use $s like the ZM_SERVER_HOST branch.

Also make the 'no corresponding entry in Servers table' errors reachable
($thisServer starts as an empty Server object, which is always truthy,
so test its Id) and concatenate the ZM_SERVER_ID error with '.' instead
of '+', which throws a TypeError on PHP 8.

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-04 08:53:28 -04:00
nicandClaude Opus 5.5 f362efd23e fix: make the Control view send PTZ to the monitor's own server (#5172)
The standalone Control view's controlCmd() still had its 2020
(control, event, xtell, ytell) signature, but ptzControls() binds the
buttons as data-on-mousedown/mouseup handlers that receive the event,
and it read the monitor id with $j('#mid').getAttribute(), which jQuery
objects don't have. Every PTZ button threw before sending anything.

Port the watch view's event-based controlCmd(): the button's value is
the command, mouseup sends moveStop, xge/yge come from data-xtell and
data-ytell. Send requests to the monitor's server (monitorUrl, from
Monitor::UrlToIndex() as on the watch view) instead of this server: on a
multi-server install only the owning server's zmcontrol.pl can drive
the camera, and relaying through Monitor::sendControlCommand() puts the
raw zmcontrol arguments into a daemonControl API path, which fails.

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-04 08:52:10 -04:00
nicandClaude Opus 5.5 72922bb459 fix(api): soft delete monitors like the console does (#5176)
DELETE /monitors/:id hard-deleted the Monitors row through CakePHP.
Zones went with it, but the monitor's Events, Monitor_Status and
Event_Summaries rows were left pointing at a monitor that no longer
exists. The console deletes with ZM\Monitor::delete(): it stops zmc
and zmcontrol and marks the monitor Deleted, keeping its events
("Events will age out") and letting the monitor be undeleted.
Use the same call in the API.

refs #5175

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-04 08:50:19 -04:00
nicandClaude Opus 5.5 ed8d648efa fix(api): key States by Id so view, edit and delete work (#5173)
The State model still declared primaryKey = 'Name' from before
zm_update-1.28.99 gave States an Id primary key. The REST routes only
match numeric ids, so states/<id>.json view/edit/delete looked up a
state *named* <id> and always failed, and names don't match the routes.
Through states/edit/<name>.json, edit never set the record id and
inserted a new nameless state instead of updating; view had no
_serialize and returned 500.

Use the default Id key. view serializes the state; edit sets the id,
accepts POST/PUT only and, like add, answers {message: Saved} or the
validation errors (as Monitors and Zones do) instead of an empty flash.
Names must be non-empty and unique, since zmpkg.pl and
states/change/<name> select states by name. index, change and delete
are unchanged.

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-03 17:44:00 -04:00
IgorA100 1eb1caf569 #alarmCues .level-graph must not extend beyond the boundaries of the parent block. (event.css) (#5177) 2026-10-03 17:29:49 -04:00
Isaac ConnorandClaude Opus 5.5 6fa81fb063 fix: key montage review frames by FrameId now that Frames.Id is gone fixes #5164
loadFrames() stored each event's frames in FramesById[frame.Id] and linked
PrevFrameId/NextFrameId through frame.Id. With the surrogate Id column
dropped, every frame landed in the same undefined slot, the time lookup
found no frame and montage review showed black. FrameId is unique within
an event, and FramesById is per event, so key and link by FrameId.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-03 10:18:12 -04:00
Isaac Connor 106e72fb56 Merge branch '5164-frames-composite-pk' 2026-10-02 15:01:48 -04:00
Isaac ConnorandClaude Opus 5.5 a6391726e1 fix: keep the event tag input visible when the stats are hidden refs #5163 (#5170)
710c6f07e moved the tags-container into the eventStats column, so
everything that hid that column (the narrow-screen auto-hide in
onStatsResize, the Stats button, and the zmEventStats=off cookie on load)
took the tag input with it. On phones the stats are always auto-hidden,
so tags could not be added at all.

Wrap the stats table, location map and alarm frames in eventStatsDetails
and hide only that. A new showEventStats() switches the column to
col-sm-12 when the details are hidden, so the tag bar spans the width
above the video as it did before 710c6f07e, and back to col-sm-3 beside
the col-sm-9 video when they are shown.

The fullscreen double-click handler in skin.js still swapped col-sm-8,
which 710c6f07e missed, leaving both col-sm-9 and col-sm-8 on the video
after leaving fullscreen. It now uses col-sm-9 and restores the layout
through showEventStats() based on whether the details are showing,
instead of forcing the stats back on.

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-02 08:59:00 -04:00
IgorA100 84925faa7c Update audio-motion element visibility in Monitor.php (#5169) 2026-10-02 08:31:12 -04:00
nicandClaude Opus 5.5 ad3d5ec54d fix: random per-user salts in zmupdate.pl password migration, rehash migrated passwords on every login (#5167)
* fix: give each migrated password its own random bcrypt salt

migratePasswords() stored the Bytes::Random::Secure object rather than
bytes from it, so en_base64() encoded the string
"Bytes::Random::Secure=HASH(0x...)". bcrypt only reads the first 22
characters of the salt, all from the constant class name, so every user
on every install got the same salt. With neither Bytes::Random::Secure
nor Data::Entropy installed the salt was empty and bcrypt() died with
"bad bcrypt settings", aborting zmupdate.pl. Even the Data::Entropy path
reused a single salt for every user in the run.

Read 16 bytes per user from /dev/urandom, as generateAuthHashSecret()
in ZoneMinder::Config already does. If it can't be read, leave that
user's legacy hash in place, which auth.php still verifies, instead of
writing a weak one. This drops the need for Bytes::Random::Secure and
the deprecated Data::Entropy. refs #4333

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* chore: drop the Data::Entropy dependency from packaging

zmupdate.pl no longer uses Data::Entropy, which upstream has deprecated
(CVE-2025-1860). refs #4333

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix: rehash legacy passwords on every password login

Only the login form called migrateHash(), so accounts that sign in with
user=/pass= or through the API kept their mysql or zmupdate.pl
(mysql+bcrypt) hash, including those with the fixed salt. Call it from
those paths too, and have it check the password type itself.

migrateHash() also regenerated the auth hash from the in-memory user,
which still held the old password, so getAuthUser(), which checks
against the new one in the database, rejected it. Update the in-memory
password first, and update the row by Id rather than by the username
as typed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix: note passwords still on a zmupdate.pl-migrated hash

Every salt migratePasswords() used before this branch was shared by all
users in a run: from Data::Entropy, which before 0.008 keys its generator
from rand() (CVE-2025-1860), or, since 38c0f743 with Bytes::Random::Secure
installed, a constant. The original hash input is gone, so zmupdate.pl
can't rehash these, and nothing distinguishes them from properly salted
ones. Log a warning listing every user still on a -ZM- hash, noting that
it is upgraded at their next login or password reset, and that unused
accounts can be disabled or deleted.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-02 08:30:43 -04:00
IgorA100 05585246ee Fix: Player selection on the Zone page (#5168)
* Fix: Player selection on the Zone page

* Refactor control buttons layout in zone.php
2026-10-02 08:24:19 -04:00
Isaac ConnorandClaude Opus 5.5 c4ef3fdd62 feat: key Frames by (EventId, FrameId) and drop the surrogate Id refs #5164
Every query on Frames filters on EventId and orders or ranges by FrameId,
and nothing needs a frame by Id. The Id primary key was a clustered index
nothing read, and EventId_FrameId_idx was a secondary index every query
used. On a copy of a local table that secondary index was half the
table's size. With (EventId, FrameId) as the primary key the index is
gone, and an event's rows are stored together, so deleting an event is a
range delete.

zm_update-1.39.36.sql:
- converts AI_Detections.FrameId from Frames.Id to the per-event frame
  number, drops its foreign key to Frames and indexes (EventId, FrameId).
  A composite foreign key cannot replace it: ON DELETE SET NULL would
  have to null the NOT NULL EventId, and Frames rows are written in
  batches, so a detection can be recorded before its frame row.
- removes duplicate (EventId, FrameId) rows, keeping the earliest.
- rebuilds Frames with the new primary key.
- removes ON UPDATE CURRENT_TIMESTAMP from Frames.TimeStamp. Any UPDATE
  of a frame row was overwriting its capture time.
Each step checks the current schema first, so the migration can be
re-run.

REST API: view, edit and delete take /frames/<action>/<EventId>/<FrameId>.json.
The old single-Id URLs return 404. CakePHP 2 has no composite keys, so
the model's primaryKey is EventId. That keeps Event's dependent cascade
delete limited to the event's own frames. The controller writes with
explicit (EventId, FrameId) conditions instead of save(), which would
match rows on EventId alone. Edit no longer changes EventId or FrameId.

view=image with fid but no eid used to look up Frames.Id. It now returns
404.

The Perl Frame class is identified by (EventId, FrameId).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-01 11:48:26 -04:00
Isaac ConnorandClaude Opus 5.5 b79239940f fix: scope the frames list search to its event and match on FrameId refs #5164
ajax/frames.php collected Frames.Id values from the event's rows and then
filtered with a FrameId IN (...) term, which FilterTerm translated to
Id IN (...). The search query had no EventId condition of its own. Once
the term matches on FrameId, which is only unique within an event, it
returns rows from every event, so the query is now anchored on the event
with the same WHERE EventId clause as the unfiltered list.

FilterTerm now maps FrameId to the FrameId column. The frames list
thumbnail links address the image by eid and fid instead of Frames.Id.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-01 11:48:26 -04:00
IgorA100 eb1d176790 Russian translation update 2026-09-30 17:46:22 +03:00
Isaac Connor 01f0bc73ac Merge pull request #5159 from IgorA100/patch-743759
Fix: Change the style of the last button in .tags-container on Event page
2026-09-29 18:31:35 -04:00
Isaac Connor 43c04115e4 Merge pull request #5160 from IgorA100/patch-750041
Fix: Enable horizontal scrolling for table on the Options->Menu page.
2026-09-29 18:31:15 -04:00
Isaac Connor 3c6d0a1f69 Merge pull request #5161 from IgorA100/patch-549294
Fix: Apply `pointer` cursor to the `#progressBar` wrapper instead of `#alarmCues` on Event page.
2026-09-29 18:30:49 -04:00