Claude/fcm token expiration renewal q15761 - #76
Merged
Conversation
… controls The "No sysmond is connected" card already replaced the editors, but the header above it - the Structured/Map/Raw toggle, the disabled picker, the mode blurb - stayed visible with nothing it could act on. It now hides under the same condition the editors use (shown while the fleet is still loading, so the page does not flash empty on every visit), leaving the card as the whole page until a box dials in. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NpxQtrW7f2YaKiYL6evoTZ
The config page and the map defaulted to the first connected site, which is fine with one box and dangerous with several: three boxes minted and one online meant an edit aimed at nothing in particular landed on whichever box happened to be up. Minted, not connected, is the count that measures ambiguity - so /api/sites now reports how many boxes hold unrevoked tokens, and the pages auto-select only when that count is one. With more than one, nothing is loaded or drawn until a box is chosen: the config page shows a chooser card (one button per connected box), the map shows its notice, and both pickers grow a "Choose a box" placeholder so the select does not display a site nobody selected. A choice goes through pickSite as ever - stored, then a full reload - so the gate at page init is the single choke point. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NpxQtrW7f2YaKiYL6evoTZ
Not everything with something to say is a monitoring box. A backup
job, a UPS script, a RAID monitor - they need to page somebody with
the same urgency tiers a sysmond's host transitions carry, and they
have no fleet of hosts behind them. They can now connect to the same
TLS listener with the same kind of minted token, greet with ALERTER
instead of HELLO, and from there the conversation inverts: nothing is
polled, the peer sends alerts.
The protocol is three verbs, documented for implementors in
docs/ALERTERS.md (shell and Python examples included):
ALERTER <name> <token> [application name...]
ALERT <CRITICAL|WARNING|OK> <object> <text...> -> 333 ok
PING / QUIT
Alerts ride the exact push pipeline host transitions ride: CRITICAL
loud, WARNING and OK quiet, <alerter>:<object> as the collapse key so
a clearing OK replaces the page it clears. The master push switch is
honored; fan-outs land in the push log like any other. Alerter alerts
carry no badge count - they have no view of the fleet's unacked total,
and a made-up number would wrongly clear the phones' icons (notifyAll
learns badge < 0 = say nothing).
Three names, in the order alerts display them: the admin's optional
nickname (the minted token's label, editable from the Fleet page and a
new /api/alerters/nickname endpoint), what the application declared
about itself at handshake, then the token name. The token name stays
the identity everywhere - collapse keys, logs, registry - so a rename
never re-keys anything.
The Fleet page gains an Alerters card - name, what it shows as, state,
address, alert count, last alert - deliberately outside the fleet
table: an alerter has no config, no hosts, no generations. For the
same reason the token record now learns its peer's kind at handshake,
and alerter tokens stop counting toward the config editors' pick-a-box
ambiguity rule. The mint modal points alert-only peers at their
greeting line; token and CA work identically for both kinds.
Tested end to end: a pipe-driven protocol session (handshake handoff,
alert parsing, 444s that don't cost the connection, nickname beating
application name, the registry the card reads) and the push fan-out
against the fake FCM endpoint, enabled and disabled.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NpxQtrW7f2YaKiYL6evoTZ
The alerter feature reviewed well enough to keep but not to ship as it was. This addresses everything the review raised, in one pass because most of the findings share files. The protocol loop no longer holds the 333 reply hostage to the push fan-out: each connection gets a small buffered queue and one dispatcher goroutine, so a slow FCM round-trip cannot make a well-behaved client time out and resend. When the queue fills, the newest alert is dropped with a log line rather than blocking the socket. Lines are read through a bounded reader that keeps at most 4096 bytes of any one line in memory, and the docs now state that bound. A reconnect no longer risks being marked dead by its predecessor: only the record's current connection may declare it disconnected. Truncation is rune-safe everywhere it happens (alert text, the handshake's application name, the nickname endpoint) via one exported TruncateRunes, instead of byte slicing that could split a UTF-8 sequence. LastSeen and LastAlertAt are pointers so JSON omits them until they mean something, and Alerters() sorts with sort.Slice. A token now belongs to the kind that first used it: the handshake refuses a sysmond token greeting as an alerter and the reverse, with a 444 naming the owner, and the kind is written to settings only when it actually changes rather than on every reconnect. The nickname endpoint only accepts names whose token is an alerter's, so it cannot be used to edit a monitoring box's label. The Fleet page fetches its three endpoints concurrently and keeps whichever sections load when one fails, with a stale-state banner that now says parts of the page may be old. On the daemon side, the ping helper probe ran access(X_OK) while still root, which answers yes for a helper the post-drop identity cannot run. It now stats the helper and requires setuid root plus an execute bit that reaches the drop user (world, or group with a matching gid), with distinct log lines for a missing helper versus one with unusable permissions. Tests cover the new behavior: the session test waits on the now asynchronous sink, and new tests pin the reconnect race, the line bound, rune-safe truncation, and the kind-claim rules. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NpxQtrW7f2YaKiYL6evoTZ
sysmond never had anything newer than the RedHat SysV script in misc/, so it gains the two files a current deployment actually wants: an OpenBSD rc.d script (misc/rc.d/sysmond) and a systemd unit (misc/sysmond.service). Both lean on what the daemon already does right: they start it as root and let it drop privileges itself (which is why neither sets a user), they run the config through sysmond -t before starting so a typo fails the start instead of coming up monitoring nothing, and reload goes through "sysmond reload", which re-parses the config and only HUPs the daemon when it passes. The systemd unit runs it with -d so systemd supervises the real process, and leaves NoNewPrivileges off deliberately - the setuid ping helper is the fallback for kernels that refuse raw-socket sends after the drop, and NoNewPrivileges would defeat it exactly when it is needed. The sysmon-web files that already existed were both broken: they still passed -sysmon, a flag that no longer exists, and Go's flag parser exits on an unknown flag - so both the unit and the rc.d script killed the process at startup. The dead flag is gone from both. The unit also loses its hard Requires on sysmond.service (monitoring boxes dial in to this process, so the usual deployment has no local sysmond at all), points its Documentation line at docs/WEB_DEPLOYMENT.md instead of a README that is never installed, and marks /etc/sysmon.conf optional in ReadWritePaths so an aggregator-only host without that file can still start the service. README's quick start now points at all four files. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NpxQtrW7f2YaKiYL6evoTZ
The fleet-wide key "site:host" was doing double duty as a display string: the web history and hosts pages and both apps' history screens printed it verbatim, so every row on a multi-box install read like branch2:coreswitch. The key keeps its job - identity, collapse, storage - but no display renders it any more. Every surface now shows the bare host name with the owning box as a separate, deliberately small element, and a single-box install shows no site at all. History events now carry site and local_name alongside the qualified object_name, filled in at append time from the host record; rows written before the split are backfilled on the way out of the store by splitting the stored key, so old and new rows serve the same shape (a test pins all three cases). The web history page gets a narrow muted Box column between When and Host that exists only when some event belongs to a named site; the inline chip that used to crowd the host cell is gone. The hosts page cards title themselves with the bare name plus a tiny site tag, keeping the full key in the tooltip. Both apps parse the new fields and render the same way: a small muted SiteTag pill (Compose and SwiftUI each get one) after the name in history rows, host rows, and the Android detail sheet. Against an older server the fields are simply absent and the apps fall back to the qualified name, so nothing breaks in either direction. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NpxQtrW7f2YaKiYL6evoTZ
Two blind spots in the structured config editor, both of the same kind: the page knew something and did not say it. It always tracked whether the working copy differed from what was loaded, but showed nothing - so nothing on screen said edits were sitting in the tab. Now an amber "Unsaved changes" pill sits next to the Save button (which is disabled when there is nothing to save and ringed when there is), the raw editor's existing note turns amber to match, and leaving the page with unsaved edits in either editor gets the browser's confirmation prompt instead of silent loss. Validation was worse: "All hosts must have IP address and check type specified" named no host, and the host table actively hid the problem by printing a "ping" badge for hosts with no check type at all - the row looked complete while save refused it. Validation now names each offending host and exactly what it is missing, in a banner instead of an alert; the bad rows sort to the top of the host list, tinted red with a "Missing IP address - edit to fix" line; the section opens, the search filter clears, and the page scrolls there so the rows are actually visible. The marks are live: fixing a host clears its row immediately, fixing the last one clears the banner. A host with no check type now shows a red "none" badge instead of a fictional "ping", and the host modal's own refusal names the specific missing field. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NpxQtrW7f2YaKiYL6evoTZ
Eight findings, worst first, all addressed. The pre-auth greeting on the public agent port was still read with an unbounded ReadString - the exact memory-exhaustion hole the alerter hardening closed everywhere else, open to unauthenticated peers. The handshake now reads through the same bounded reader as the rest of the protocol, so a newline-free flood costs 4096 bytes, not everything the peer can push before the 20-second deadline. The privilege drop kept root's supplementary groups (setgid and setuid do not touch them), which both weakened the drop and made the new ping helper probe dishonest - a 4750 root:root helper stayed executable post-drop via a retained group while the probe declared it unusable and disarmed the fallback. revoke_root now calls setgroups first, while still root, so the process ends up holding exactly the drop user's gid and the probe's primary-gid check matches reality. A full alerter delivery queue answered "333 ok" while dropping the alert - a delivery promise for a page that was never sent, and a compliant client would never resend it. It now answers "444 busy" so the client retries, refused alerts are not counted, and ALERTERS.md documents that 333 is the only reply that means accepted. claimKind's check-then-set also became one store transaction (ClaimAgentKind), so two racing first handshakes cannot claim a fresh token as different kinds; tests cover both the queue refusal end to end and the race. The systemd unit claimed /etc/sysmon.conf was writable "so the config editor can save", but the save writes a temp file and renames, which needs a writable /etc directory no file-level carve-out can grant. The useless entry is gone and the unit (and WEB_DEPLOYMENT.md) now say plainly: local editor saves need a drop-in widening the sandbox; the fleet flow over the agent link is unaffected. Display stragglers: the hosts page list view and detail modal still printed the raw "site:host" key - both now show the bare name with the small site tag like everywhere else. The Android history and host rows ellipsize long names instead of shoving the site tag off-screen, and the history page computes its Box-column flag once per fetch instead of once per rendered cell. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NpxQtrW7f2YaKiYL6evoTZ
Every alert(), confirm() and prompt() in the UI - 82 call sites across ten pages - went through the browser's native dialogs, which look like the browser, not the page, and block the event loop. base.html now ships a small dialog kit (uiAlert, uiConfirm, uiPrompt): an in-page overlay card styled like the rest of the UI, promise-based, queued so a second dialog waits for the first, with Enter/Escape and backdrop handling and a red confirm button for destructive questions. All call sites are converted; confirm and prompt sites became awaited calls in async handlers, and uiPrompt keeps native prompt's (message, initial) signature so the conversions stay mechanical. The config editor gets the guard the unsaved-changes pill was pointing at: clicking any in-app link, or switching boxes in the site picker, with unsaved edits in either editor now raises a styled "Unsaved changes - discard and leave, or stay?" dialog before anything is lost; staying snaps the site picker back. The browser's generic beforeunload prompt remains only as the backstop for tab closes and external navigation, where the platform forbids custom UI, and it stands down once the styled dialog has already been answered with "discard". The committed Tailwind build is regenerated, which also picks up classes added in the last several UI commits (the arbitrary-size text utilities, the ring on the save button) that were missing from the committed stylesheet and would have rendered unstyled. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NpxQtrW7f2YaKiYL6evoTZ
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 1bdc006. Configure here.
| perror("revoke_root: setgroups"); | ||
| print_err(1, "WARNING: Failed to drop supplementary groups"); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Failed group drop keeps root
Medium Severity
If setgroups fails, revoke_root_if_necessary returns after a warning and never calls setgid/setuid, so the daemon keeps running as root. That failure path is new with this change and is worse than still attempting the uid/gid drop.
Reviewed by Cursor Bugbot for commit 1bdc006. Configure here.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Note
Overview
Adds alerters: alert-only peers (backup jobs, UPS scripts, etc.) that dial the same TLS agent port as sysmond, greet with
ALERTER, and inject CRITICAL/WARNING/OK into the existing push pipeline without joining the fleet.Tokens are bound to a kind at first handshake (
sysmondvsalerter) so they cannot switch roles. Alerts are queued with a busy refusal, bounded line reads, and collapse keys ofsource:object. New APIs list alerters and set nicknames;docs/ALERTERS.mdis the protocol.Android/iOS (and history) now show a quiet site tag plus the bare name instead of
site:host. Single-boxlocalinstalls look unchanged.sysmond now drops supplementary groups, probes the ping helper as the post-drop identity, and ships systemd/OpenBSD rc.d units. The web UI replaces native
alert/confirm/promptwith in-page dialogs.Reviewed by Cursor Bugbot for commit 1bdc006. Bugbot is set up for automated code reviews on this repo. Configure here.