feat: Add custom model training annotation UI

[pliablepixels]

Summary

• Built-in annotation editor on the event view for correcting object detection results
• Saves training data in YOLO format for custom model training via pyzm
• Two-panel training data browser with preview and inline delete
• Frame navigation with thumbnails, skip, pagination
• Gated behind ZM_OPT_TRAINING config option — zero impact when disabled

Refs #4666

Test plan

• Enable ZM_OPT_TRAINING in Options > Config
• Open an event, click the Annotate button
• Draw boxes, label objects, save annotations
• Run detection, accept/reject results
• Navigate frames (prev/next, skip, browse thumbnails)
• Browse training data folder, preview files, delete
• Verify unsaved-changes warning on navigation
• Verify no UI changes when ZM_OPT_TRAINING is disabled
• Screenshots will be posted on feat: Add custom model training annotation UI to event view #4666

🤖 Generated with Claude Code

[Copilot]

Pull request overview

Adds a new, config-gated (“ZM_OPT_TRAINING”) annotation workflow in the classic skin event view to correct object detections and persist training data in YOLO format for custom model training.

Changes:

• Adds a canvas-based annotation editor UI to the event view, including frame navigation and an overlay-based training-data browser.
• Introduces a new AJAX backend (web/ajax/training.php) to load/save annotations, browse/delete dataset files, and optionally run an external detection script.
• Adds new configuration options (DB migration + ConfigData + translations) and bumps version to 1.39.1.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
web/skins/classic/views/js/training.js	New AnnotationEditor implementation (drawing/editing boxes, browsing frames/data, saving YOLO).
web/skins/classic/views/js/event.js.php	Initializes the editor and wires UI handlers when ZM_OPT_TRAINING is enabled.
web/skins/classic/views/event.php	Adds the Annotate button, annotation panel markup, and loads training CSS/JS when enabled.
web/skins/classic/css/base/views/training.css	Styles for the annotation panel, frame browser, and training-data browser overlays.
web/ajax/training.php	Backend endpoints for training editor (load/save/status/browse/delete/detect).
web/ajax/modals/objdetect.php	Adds a shortcut button into the objdetect modal to open the annotation UI.
web/lang/en_gb.php	Adds translation keys for the new feature and new config option prompts/help.
scripts/ZoneMinder/lib/ZoneMinder/ConfigData.pm.in	Adds new options ZM_OPT_TRAINING, ZM_TRAINING_DATA_DIR, ZM_TRAINING_DETECT_SCRIPT.
db/zm_update-1.39.1.sql	Adds Config entries for the new options.
version.txt	Version bump to 1.39.1.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The realpath containment check uses strpos($fullPath, realpath($base)) !== 0, which is vulnerable to prefix-matching paths (e.g. $base='/data/training' and $fullPath='/data/training_evil/file' passes). Use a boundary-safe check (e.g. compare against $baseReal . DIRECTORY_SEPARATOR or verify the next char is /).

[pliablepixels]

Fixed in 62678b2. Now uses rtrim(realpath($base), DIRECTORY_SEPARATOR).DIRECTORY_SEPARATOR before the strpos containment check, preventing prefix-matching against sibling directories.

[Copilot]

Same path containment issue as browse_file: strpos($fullPath, realpath($base)) !== 0 can be bypassed by sibling directories that share the same prefix as $base. Tighten the check to ensure $fullPath is actually under $base (boundary-aware).

Suggested change

	$reqPath = detaintPath($_REQUEST['path']);
	$fullPath = realpath($base.'/'.$reqPath);
	if ($fullPath === false || strpos($fullPath, realpath($base)) !== 0 || !is_file($fullPath)) {
	ajaxError('File not found or access denied');
	break;
	}
	$baseReal = realpath($base);
	if ($baseReal === false) {
	ajaxError('Training data directory not found or invalid');
	break;
	}
	$reqPath = detaintPath($_REQUEST['path']);
	$fullPath = realpath($baseReal . '/' . $reqPath);
	if ($fullPath === false) {
	ajaxError('File not found or access denied');
	break;
	}
	$basePrefix = rtrim($baseReal, DIRECTORY_SEPARATOR) . DIRECTORY_SEPARATOR;
	if (strpos($fullPath, $basePrefix) !== 0 || !is_file($fullPath)) {
	ajaxError('File not found or access denied');
	break;
	}

[pliablepixels]

Fixed in 62678b2. Same fix applied — appending DIRECTORY_SEPARATOR to the base path before strpos check.

[Copilot]

tempnam(...).'.jpg' creates (and leaks) the original temp file returned by tempnam() while writing to a different filename. This leaves stray files in the temp dir. Use the tempnam() return value directly (optionally rename it) and ensure it’s always cleaned up (also on error paths).

Suggested change

	$tmpFile = tempnam(sys_get_temp_dir(), 'zm_detect_').'.jpg';
	if (!copy($srcImage, $tmpFile)) {
	$tmpFile = tempnam(sys_get_temp_dir(), 'zm_detect_');
	if ($tmpFile === false) {
	ajaxError('Failed to create temp file for detection');
	break;
	}
	if (!copy($srcImage, $tmpFile)) {
	if (file_exists($tmpFile)) {
	unlink($tmpFile);
	}

[pliablepixels]

Fixed in 62678b2. Now renames the tempnam() base file to .jpg instead of creating a second file, so no orphan is left behind. Also added cleanup on copy failure.

[Copilot]

The migration Help text says an empty ZM_TRAINING_DATA_DIR defaults alongside events storage, but the runtime fallback is ZM_DIR_CACHE/training and ConfigData.pm.in defaults to @ZM_CACHEDIR@/training. Please align the migration help text with the actual default behavior (or change the default).

Suggested change

	Help='Filesystem path where corrected annotation images and YOLO label files are stored. The directory will be created automatically if it does not exist. Uses Roboflow-compatible YOLO directory layout (images/all/, labels/all/, data.yaml). If left empty, defaults to a training folder alongside your events storage.',
	Help='Filesystem path where corrected annotation images and YOLO label files are stored. The directory will be created automatically if it does not exist. Uses Roboflow-compatible YOLO directory layout (images/all/, labels/all/, data.yaml). If left empty, defaults to a training folder in the ZoneMinder cache directory (e.g. ZM_DIR_CACHE/training).',

[pliablepixels]

Fixed in 62678b2. Help text now says "defaults to a training folder inside the ZoneMinder cache directory" which matches the actual ZM_DIR_CACHE/training fallback.

[Copilot]

training.js updates a .frame-total element inside #annotationFrameSelector, but the markup here doesn't include any element with class frame-total, so the total frame count will never render. Add the missing element (or adjust the JS to target an existing node).

Suggested change

	<button class="btn btn-normal btn-sm" id="annotationGoToFrame"><?php echo translate('GoToFrame') ?></button>
	<button class="btn btn-normal btn-sm" id="annotationGoToFrame"><?php echo translate('GoToFrame') ?></button>
	<span class="frame-total"><?php echo $Event->Frames() ?></span>

[pliablepixels]

Fixed in 62678b2. Added the missing <span class="frame-total"></span> element to the frame selector markup in event.php.

[Copilot]

The Delete/Backspace key handler calls _pushUndo() and then calls deleteAnnotation(), which also calls _pushUndo(). This causes a single delete action to require two undos and bloats the undo stack. Remove one of the _pushUndo() calls (typically keep it inside deleteAnnotation()).

Suggested change

self._pushUndo();

[pliablepixels]

Fixed in 62678b2. Removed the extra _pushUndo() call from the Delete/Backspace keydown handler — deleteAnnotation() already calls it internally.

[Copilot]

The detect endpoint returns raw_output from the external detection script. The front-end doesn't appear to use it, and it can leak internal details or error output to the browser unnecessarily. Consider omitting it (or gating it behind a debug flag / admin-only).

Suggested change

'raw_output' => $output,

[pliablepixels]

Fixed in 62678b2. Removed raw_output from the detect response. The script output is still logged server-side via ZM\Debug for troubleshooting.

[Copilot]

The help text says an empty ZM_TRAINING_DATA_DIR defaults to a folder alongside events storage, but the configured default is @ZM_CACHEDIR@/training and web/ajax/training.php falls back to ZM_DIR_CACHE.'/training'. Update the help text (or the implementation) so the documented default matches reality.

Suggested change

	If left empty, defaults to a training folder alongside your events storage.
	If left empty (or set to the default), ZoneMinder uses a "training"
	subdirectory under the cache directory (ZM_DIR_CACHE.'/training',
	typically @ZM_CACHEDIR@/training).

[pliablepixels]

Fixed in 62678b2. Same correction as the migration — now says "inside the ZoneMinder cache directory."

[Copilot]

State-changing training endpoints like action=save are callable via ?request=training without any CSRF protection, so an attacker can forge POST requests from another site that run in a logged-in user’s browser and modify or delete training annotations. If a user with canEdit('Events') visits a malicious page while logged in, that page can silently submit a form to ?request=training&action=save/delete/delete_all/browse_delete and corrupt or wipe the training dataset. To fix this, enforce CSRF tokens or the existing csrf-magic mechanism for request=training actions and validate them on these write operations before applying any changes.

[pliablepixels]

Skipping this one. ZoneMinder's existing AJAX endpoints (event delete, monitor config, etc. in web/ajax/) do not enforce CSRF tokens either — this is a codebase-wide pattern, not specific to the training feature. Adding CSRF validation here alone would be inconsistent. This should be addressed as a separate codebase-wide effort.

[connortechnology]

I wonder if we could just output $Event->to_json() and make this a 1-liner.

[pliablepixels]

Fixed in ca74d11. Replaced with const eventData = <?php echo $Event->to_json() ?>; — single line, uses the inherited Object::to_json() method.

[connortechnology]

these should not be here

[pliablepixels]

Fixed in ca74d11. Removed both lines — the training view never reads or writes session data, so the session open/close was a no-op.

[connortechnology]

We don't need this. This wouldn't actually correctly update the config table anyways. zmupdate.pl -f will automatically add this field to config.

[pliablepixels]

Fixed in ca74d11. Deleted the migration file entirely — the three config entries are already defined in ConfigData.pm.in and zmupdate.pl -f handles syncing them to the database.