Skip to content

Home Assistant event API + Telegram menu grouping - #20

Merged
davidnewhall merged 20 commits into
mainfrom
dn2_ha_event_api
Aug 10, 2026
Merged

Home Assistant event API + Telegram menu grouping#20
davidnewhall merged 20 commits into
mainfrom
dn2_ha_event_api

Conversation

@davidnewhall

@davidnewhall davidnewhall commented Aug 9, 2026

Copy link
Copy Markdown
Owner

Summary

First half of the HA → Motifini → Telegram plan: Motifini's HTTP API learns how to register, list, and richly notify events so Home Assistant automations can fire text + camera notifications that Telegram users subscribe to. HA never talks to Telegram; Motifini keeps owning the bot.

Webserver API (pkg/webserver):

  • PUT /api/v1.0/event/{event} — upsert a catalog event (description via form field or JSON body). New events are tagged source=ha; state saved on every accepted PUT.
  • GET /api/v1.0/events — list the subscribable catalog as JSON (event, description, source, subscribers); reserved __cam: clip-settings keys excluded.
  • POST /api/v1.0/event/notify/{event} — enhanced notify:
    • msg optional text/caption, camera = SecuritySpy name or number, media = none|photo|video, optional description (used only when the event is new).
    • Defaults: camera set + media omitted → photo. Explicit media=none → text-only (camera ignored).
    • Rejects (400): no msg and no camera media; photo/video without camera; unknown media value; unknown camera; event names over 60 bytes. SecuritySpy not ready → 503.
    • Unknown events self-register (source=ha, description from body or event id) so Telegram menus update without a separate register call; if the state save fails, the registration rolls back and the notify returns 500.
    • Video clips use the admin per-camera clip settings (chat.GetCameraClipSettings/VideoClipOps), same as motion alerts.
    • Missing media is called out in the message: a known-offline camera skips capture and the note ⚠ X is offline — no photo attached. rides along (200, degraded delivery); an online-camera capture failure appends ⚠ Couldn't capture … and returns 500.
  • POST .../remove/{event} unchanged (still wipes catalog entry + all subscriptions, saves state).
  • New [webserver] listen_addr config (default 127.0.0.1) — the old localhost-only bind would make these endpoints unreachable when HA runs on another host.

Auth:

  • Optional [webserver] api_key. When set, every route (including /debug/vars) requires it via Authorization: Bearer <key>, X-API-Key: <key>, or ?apikey=<key> (for webhook callers like SecuritySpy custom actions that can't set headers; the apikey value is redacted from request logs).
  • Constant-time comparison over SHA-256 digests (no length/timing leak).
  • Required when exposed: listen_addr set to anything non-loopback with no api_key → startup fails with ErrAPIKeyRequired. Loopback bind without a key stays open, same as today.
  • New [webserver] allow_subscribers option: the /api/v1.0/send/* endpoints no longer require every chat id in the hardcoded allowed_to list — with the flag on, any authenticated, non-ignored Telegram subscriber (hasAuth via /id or admin Allow) is a valid to recipient. allowed_to still works and is checked first.
  • Companion PR HACS integration: Motifini events from Home Assistant #21 sends the key from Home Assistant.

Telegram menus (pkg/chat):

  • /events wizard and Subscribe → Event picker split the catalog into — Home Assistant — / — System — sections (inert header buttons). Sub/unsub/pause flows unchanged.
  • Subscribe buttons carry the event name (not a list index), resolved at tap time — a runtime register/remove can't mis-subscribe anyone. Labels truncate on rune boundaries.

⚠️ Breaking change

notify no longer attaches a JPEG when the event name happens to equal a camera name, and a bare notify with neither msg nor camera media is now 400 instead of sending the event id as text. If you have SecuritySpy (or anything else) POSTing event/notify/{cameraName}, add ?camera={cameraName} to keep photos flowing. (Auth is backward-compatible: with no api_key configured and a localhost bind, nothing changes.)

Copilot review — addressed

  • ?apikey= credentials persisted in request logs → redacted.
  • PUT save failure → identical retry skipped the save → PUT now saves every time.
  • Notify auto-register save failure → event lost on restart → now rolls back + 500.
  • Index-based event callbacks could mis-subscribe after runtime catalog changes → name-based callbacks + 60-byte registration cap.
  • Byte-based label truncation could split UTF-8 → rune-based.
  • No test coverage past the SecuritySpy-not-ready 503 → fake SecuritySpy server covers name/number resolution, unknown-camera 400, photo capture, offline shortcut, and capture failure.

Concurrency fixes

Three data races, all confirmed with the race detector and each covered by a test that fails without the fix:

  1. Subscriber records (pkg/chat/substate.go). Meta, Contact, Admin and Ignored were read and written with no coordination, so a state file save could race a Meta write and panic with concurrent map read and map write. The exposure was wider than the new HTTP path: Telegram dispatches every callback in its own goroutine (go m.handleTelegramCallback), so two chat callbacks raced each other too. A guarded accessor layer here closed motifini-versus-motifini, but not motifini-versus-library: GetSubscribers filters on Ignored, GetAdmins on Admin, GetSubscriber compares Contact, and the save marshals all four, none of them holding motifini's lock. Fixed upstream in Add per-subscriber lock and accessors for Meta/Contact/Admin/Ignored golift/subscribe#32, which guards the four fields per record; the helpers here now delegate to its accessors.
  2. Event catalog transactions (registerNotifyEvent). The check-create-save-rollback sequence now runs under one catalog mutex, shared with the PUT upsert and the remove handler. Unserialized, two requests could both find an event missing; the one whose save failed would delete the event the other had already persisted, and a peer could answer 200 for an event that was never written.
  3. SecuritySpy camera list. Refresh() replaces Server.Cameras and Server.Info and runs from the retry loop, the event stream and the Telegram /refresh command, while request goroutines, chat wizards and the /debug/vars callbacks read those fields. The library asked callers to lock an unexported mutex, so there was no safe read. Fixed upstream in Add locked accessors for the fields Refresh() replaces golift/securityspy#94 (GetCameras() / GetInfo() / GetGroups()); this PR uses them.

Dependency note: golift.io/securityspy/v2 is on the released v2.2.0. golift.io/subscribe is pinned by commit SHA to golift/subscribe#32 and stays that way — that library is not tagged for release, so a pseudo-version is the intended, permanent form here. Merge #32 first so the pinned commit is on main.

Judgment calls (please review)

  • Plan's combination table said "msg set + media omitted → text-only", but the plan's defaults also said "camera set + media omitted → photo". I followed the defaults line: msg + camera + no media sends photo + caption. Explicit media=none is the way to force text-only.
  • Known-offline camera → 200 (delivery succeeded, degraded, note in the message); capture failure on an online camera → 500 so the automation notices.
  • PUT on a pre-existing event without a source rule claims it as source=ha (upsert semantics); notify auto-register only touches missing events, so built-ins never get re-tagged by notifies.
  • listen_addr wasn't in the plan, but without it the whole HA flow only works when HA and Motifini share a host. Localhost stays the default; exposing it requires api_key.

Test plan

  • go test -race ./... — includes four concurrency tests (subscriber state under simultaneous read/write/save, the event API hammered by parallel register/notify/list/remove, all-fail rollback under a broken state file, and camera lookups during a refresh). Also covers upsert (form+JSON, save-every-time), list (excludes __cam:), the full notify validation matrix, auto-upsert/description immutability, save-failure rollback, remove wiping subs + state file, key extraction/constant-time compare/redaction, middleware allow/deny, startup rejection of non-loopback-without-key, recipient authorization, offline/capture-failure notes, and an end-to-end pass asserting /api/* and /debug/vars require the key.
  • golangci-lint run ./... — 0 issues; builds on darwin/linux/windows/freebsd.
  • Live check against real SecuritySpy + Telegram: register → subscribe via /events → notify photo/video/text, with and without the key.

davidnewhall and others added 3 commits August 9, 2026 12:48
New routes so Home Assistant (or any HTTP client) can drive event
notifications without touching Telegram:

- PUT /api/v1.0/event/{event} upserts a catalog event (description via
  form field or JSON body). New events are tagged source=ha and the
  state file is saved after each mutation.
- GET /api/v1.0/events lists the subscribable catalog as JSON
  (description, source, subscriber count), excluding reserved __cam:
  clip settings keys.
- POST /api/v1.0/event/notify/{event} now takes msg, camera, and media
  (none|photo|video) form fields. Camera resolves by SecuritySpy name
  or number; media defaults to photo when a camera is given. Unknown
  events self-register (source=ha) so Telegram menus pick them up.
  Video clips use the admin per-camera clip settings.

Breaking: notify no longer attaches a JPEG when the event name happens
to equal a camera name, and a notify with neither msg nor camera media
is now rejected (400) instead of inventing a caption from the event id.

Co-authored-by: Cursor <cursoragent@cursor.com>
The /events wizard and the Subscribe -> Event picker now split the
catalog into Home Assistant (source=ha) and System sections with
inert header rows. Subscribe callbacks index into the shared
EventMenuNames ordering so both menus stay in sync.

Co-authored-by: Cursor <cursoragent@cursor.com>
The API bound to 127.0.0.1 only, which makes the new event endpoints
unreachable when Home Assistant runs on another host. listen_addr keeps
the localhost default and lets a trusted LAN address (or 0.0.0.0) be
configured explicitly.

Co-authored-by: Cursor <cursoragent@cursor.com>
When [webserver] api_key is set, every route (including /debug/vars)
requires the key as Authorization: Bearer, X-API-Key, or ?apikey= for
webhook callers that cannot set headers. Keys compare in constant time
via SHA-256 digests. With no key configured the API stays open, which
remains fine for the localhost-only default; binding a non-loopback
listen_addr without a key logs a loud startup warning.

Co-authored-by: Cursor <cursoragent@cursor.com>
davidnewhall added a commit that referenced this pull request Aug 9, 2026
The config flow collects an optional api_key (password field) and the
client sends it as Authorization: Bearer on every request. A 401/403
during setup now surfaces as an invalid-auth error instead of a generic
connection failure. Pairs with the server-side api_key in #20.

Co-authored-by: Cursor <cursoragent@cursor.com>
davidnewhall and others added 2 commits August 9, 2026 13:06
Start() launches the listener in a goroutine, so the previous test raced
it and flaked in CI with connection refused. Extracting handler() lets
the test exercise the same routed, auth-wrapped stack synchronously.

Co-authored-by: Cursor <cursoragent@cursor.com>
A non-loopback bind without a key now fails startup with
ErrAPIKeyRequired instead of just logging a warning.

Co-authored-by: Cursor <cursoragent@cursor.com>
davidnewhall added a commit that referenced this pull request Aug 9, 2026
Matches the server-side startup enforcement in #20.

Co-authored-by: Cursor <cursoragent@cursor.com>
@davidnewhall
davidnewhall requested a balanced review from Copilot August 9, 2026 20:31

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds Home Assistant event registration and media notifications while keeping Telegram subscription management in Motifini.

Changes:

  • Adds event catalog and rich notification HTTP APIs.
  • Adds API-key authentication and configurable network binding.
  • Groups Telegram events by Home Assistant and system source.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
pkg/webserver/webserver.go Adds routes, bind configuration, and camera lookup.
pkg/webserver/webhandler_events.go Implements event catalog and notification behavior.
pkg/webserver/webhandler_events_test.go Tests event API behavior.
pkg/webserver/auth.go Implements API-key authentication.
pkg/webserver/auth_test.go Tests authentication and bind restrictions.
pkg/motifini/start.go Connects new webserver configuration.
pkg/chat/wizard.go Uses grouped event ordering.
pkg/chat/wizard_cmds.go Adds grouped event menu callbacks.
pkg/chat/eventgroups.go Builds Home Assistant and system event sections.
pkg/chat/eventgroups_test.go Tests event grouping and menu ordering.
examples/motifini.conf.example Documents bind address and API key settings.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread pkg/webserver/auth.go Outdated
return key
}

return request.URL.Query().Get("apikey")
Comment thread pkg/webserver/webhandler_events.go Outdated
Comment thread pkg/webserver/webhandler_events.go Outdated
Comment thread pkg/chat/eventgroups.go Outdated
Comment thread pkg/chat/eventgroups.go Outdated
Comment thread pkg/webserver/webhandler_events.go
davidnewhall and others added 5 commits August 9, 2026 13:37
The /api/v1.0/send/* endpoints required every Telegram chat id in the
hardcoded allowed_to list. With allow_subscribers = true, any
authenticated, non-ignored Telegram subscriber (hasAuth via /id or
admin Allow) is also a valid recipient. allowed_to still works and is
checked first.

Co-authored-by: Cursor <cursoragent@cursor.com>
The parallel subtests mutated a shared Config.AllowSubscribers; give
each subtest its own Config over the shared (read-only) subscriber DB.
Caught by CI's race detector.

Co-authored-by: Cursor <cursoragent@cursor.com>
Copilot review on #20: finishReq/finishReqJSON logged
request.URL.String() verbatim, so callers using the documented
?apikey= option persisted the secret in the application log. The
apikey query value is now REDACTED in log lines.

Co-authored-by: Cursor <cursoragent@cursor.com>
Copilot review on #20:

- Event subscribe buttons carried an index into a freshly recomputed
  catalog list, so a runtime register/remove while a menu was open could
  subscribe the user to the wrong event. Buttons now carry the event
  name, resolved case-insensitively at tap time. Registration caps event
  names at 60 bytes so they always fit Telegram's 64-byte callback_data
  (with the e:s:/s:e: prefixes); legacy over-long names are hidden from
  the menus with a note instead of breaking the keyboard.
- Menu label truncation operated on bytes and could split a multi-byte
  UTF-8 character. Truncation is now rune-based.

Co-authored-by: Cursor <cursoragent@cursor.com>
Copilot review on #20:

- A failed StateFileSave after a PUT upsert left the catalog mutated in
  memory, and an identical retry skipped the save and returned 200 with
  the state file still stale. PUT now saves on every accepted request.
- A failed save after notify auto-registration was only logged, so the
  new event vanished on restart and later notifies never retried. The
  registration is now rolled back and the notify returns 500.
- The notify tests stopped at the 503 path; a fake SecuritySpy server
  now covers camera resolution by name and number, unknown-camera 400,
  and photo capture via ++image.

Feature: when the requested camera is known-offline, the capture is
skipped and the notify goes out with "X is offline — no photo attached."
appended (200: delivery succeeded, degraded). A capture failure on an
online camera appends a similar note and still returns 500.

Co-authored-by: Cursor <cursoragent@cursor.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.

Suppressed comments (5)

pkg/webserver/auth.go:60

  • This only redacts when the first decoded apikey value is non-empty. A valid URL such as ?apikey=&apikey=secret makes Query().Get return "", so the full URL—including secret—is logged. Check whether the key exists in the query map instead; query.Set will then replace all duplicate values.
	if query.Get("apikey") == "" {
		return request.URL.String()

pkg/webserver/webhandler_events.go:239

  • Notify auto-registration has the same invalid-UTF-8 round-trip problem as PUT: JSON persistence and Telegram callbacks replace invalid bytes, leaving a catalog entry callers cannot address consistently. Reject invalid UTF-8 here as well.
	if req.event == "" || len(req.event) > chat.MaxEventNameLen {
		return nil, http.StatusBadRequest, "ERROR: invalid event name\n"

pkg/webserver/webhandler_events.go:95

  • Media types are case-insensitive, and this prefix check also misclassifies values such as application/json-patch as JSON. A standards-compliant Application/JSON request is currently treated as form data and silently loses its description. Compare the media type (before ; parameters) case-insensitively and exactly.
	if !strings.HasPrefix(request.Header.Get("Content-Type"), "application/json") {
		return request.FormValue("description"), nil

pkg/webserver/webhandler_events.go:54

  • This accepts percent-encoded invalid UTF-8 in the URL path as an event name. encoding/json replaces invalid bytes when the catalog is saved/listed, so the event ID changes on restart and cannot round-trip through Telegram callback data. Reject invalid UTF-8 during registration.

This issue also appears on line 238 of the same file.

	if event == "" || chat.IsCamSettingsKey(event) || len(event) > chat.MaxEventNameLen {

pkg/webserver/webhandler_events.go:323

  • A failed SaveJPEG/SaveVideo may leave a partial file at path, but returning an empty attachment path means SendFileOrMsg never removes it. Repeated capture failures—especially retried videos—can accumulate files in TempDir; remove path best-effort in this error branch before falling back to text.
	if err != nil {
		c.Error.Printf("[%v] capture %s for %s: %v", reqID, req.media, req.event, err)
		// Text-only fallback; never attach a missing/partial file.
		note := "⚠ Couldn't capture " + req.media + " from " + req.cam.Name + "."

Comment thread pkg/webserver/webserver.go Outdated
Comment thread pkg/webserver/webhandler_events.go Outdated
davidnewhall and others added 3 commits August 9, 2026 14:22
Telegram dispatches each callback in its own goroutine and every HTTP
request runs in another, yet Meta, Contact, Admin and Ignored were read
and written with no coordination. The subscribe library treats those as
caller data and copies them while writing the state file, so a save could
race a Meta write and panic with "concurrent map read and map write".
All access now goes through guarded accessors in pkg/chat, and the state
file save holds the read lock.

The event API's register-then-save sequences also run under one lock.
Without it two requests could both find an event missing, and the one
whose save failed would roll back the event the other had just persisted.

Co-authored-by: Cursor <cursoragent@cursor.com>
Refresh() replaces Server.Cameras and Server.Info, and it runs from the
retry loop, the event stream and the Telegram /refresh command while HTTP
requests, chat wizards and the /debug/vars callbacks read those fields. The
library had no way to read them safely; golift/securityspy#94 adds
GetCameras(), GetInfo() and GetGroups(), so use them everywhere and pin the
dependency to that commit.

Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 30 out of 31 changed files in this pull request and generated 1 comment.

Suppressed comments (4)

pkg/webserver/auth.go:63

  • Checking Query().Get only examines the first value. An authenticated request such as ?apikey=&apikey=secret therefore reaches the handler but logs the raw URL, including the second credential. Redact whenever the apikey key is present, regardless of its first value.
	query := request.URL.Query()
	if query.Get("apikey") == "" {
		return request.URL.String()
	}

	query.Set("apikey", "REDACTED")

pkg/webserver/webhandler_events.go:352

  • On capture errors, path is discarded without deleting a file that SaveJPEG/SaveVideo may have partially created. Repeated camera failures can therefore accumulate orphaned media in the temp directory. Remove path before returning the text-only fallback.
	if err != nil {
		c.Error.Printf("[%v] capture %s for %s: %v", reqID, req.media, req.event, err)
		// Text-only fallback; never attach a missing/partial file.
		note := "⚠ Couldn't capture " + req.media + " from " + req.cam.Name + "."

		return appendNote(req.msg, note), "", http.StatusInternalServerError, "ERROR: " + err.Error() + "\n"

pkg/chat/wizard.go:231

  • Resolving the catalog name and subscribing are separate operations with no lock shared with the HTTP remove transaction. If EventRemove runs between them, this callback can recreate an orphan subscription to an event that was just removed; if that name is later auto-registered, the user starts receiving it unexpectedly. Make the existence-check/subscribe operation atomic with catalog removal, or add a transaction API that both chat and webserver paths share.
	// Buttons carry the event name; resolve it (case-insensitively) against the
	// live catalog so a renamed/removed event can never mis-subscribe.
	event := c.Subs.Events.Name(name)

go.mod:14

  • This still pins the unreleased SecuritySpy commit that the PR description explicitly marks as temporary and says must be replaced before merge. Update this to the tagged release containing the concurrency-safe accessors before approving the PR.
	golift.io/securityspy/v2 v2.1.3-0.20260809224124-a3ee38f91d09

Comment thread pkg/webserver/webhandler_events.go Outdated
davidnewhall and others added 2 commits August 9, 2026 16:21
…ribe race

A PUT whose state save fails now rolls a newly created event back out of
memory. Leaving it there made a later notify treat it as registered, skip
its own persistence and answer 200 for an event that vanished on restart.

Log redaction ignores case and covers repeated apikey parameters. A key
that authentication rejects -- ?apikey=&apikey=x, where Get returns only
the empty first value -- is still a credential, and the 401 is logged.

A failed capture deletes its partial file: returning no media path means
nothing downstream cleans up, so a flapping camera piled up temp files.

The event subscribe wizard re-checks the catalog after subscribing and
undoes itself if the HTTP remove endpoint dropped the event mid-flight,
which otherwise left a hidden subscription that would spring back to
life if the name were ever re-registered.

Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
The chat package guarded Meta/Contact/Admin/Ignored with a lock of its own,
which closed motifini-versus-motifini races but not motifini-versus-library:
GetSubscribers filters on Ignored, GetAdmins on Admin, GetSubscriber compares
Contact, and the state file save marshals all four, none of them holding the
motifini lock. subscribe now guards those fields per record, so the helpers
here delegate to its accessors and the local lock is gone.

subscriberDisplayName recovers a name from Meta with SetContactIfEmpty rather
than an assignment, so an admin rename that landed since the read survives.

The concurrency test now calls the library readers alongside the motifini
ones -- they are the other half of the pairing that raced.

Co-authored-by: Cursor <cursoragent@cursor.com>
davidnewhall and others added 2 commits August 9, 2026 16:58
go get -u ./...: carbon v2.6.17, pion/srtp v3.0.13, pion/transport v4.1.0.
Repins subscribe to the tip of golift/subscribe#32.

Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 30 out of 31 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

go.mod:15

  • The PR's dependency note says this temporary commit pin must be replaced by a tagged subscribe release before merge, but the changed requirement still uses a pseudo-version. Please bump to the released tag and regenerate go.sum before merging.
	golift.io/subscribe v0.0.0-20260810000412-d88353a7f062

Comment thread pkg/webserver/auth.go Outdated
Comment on lines +67 to +82
query := request.URL.Query()
redacted := false

for param := range query {
if strings.EqualFold(param, apiKeyParam) {
query.Set(param, "REDACTED") // Set drops any repeated values.

redacted = true
}
}

if !redacted {
return request.URL.String()
}

return request.URL.Path + "?" + query.Encode()
URL.Query() drops any field holding an unescaped semicolon, so
?apikey=s3cret;x=1 parsed as nothing at all, redaction found no key, and the
401 logged the secret verbatim. Walk the raw query instead, splitting on both
separators, and unescape each field name before comparing: %61pikey reaches
the handler as a working credential.

Co-authored-by: Cursor <cursoragent@cursor.com>
@davidnewhall

Copy link
Copy Markdown
Owner Author

On the go.mod pin: golift.io/subscribe is not tagged for release, so the commit pin is the intended permanent form rather than a placeholder. The PR body said otherwise; that note is now corrected. The only ordering requirement is that golift/subscribe#32 merges first, so the pinned commit lands on main.

@davidnewhall
davidnewhall merged commit 65fa573 into main Aug 10, 2026
7 checks passed
@davidnewhall
davidnewhall deleted the dn2_ha_event_api branch August 10, 2026 01:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants