Feat/ops hardening#25
Merged
Merged
Conversation
- Track all user lifecycle events (create, rename, enable/disable, limit changes, plan changes, rotation, Telegram bind/unbind, deletion) - Log payment events (created, paid, cancelled) with order details - Record self-service actions and system events (expiry, quota exhaustion, device limit, auto-downgrade) - Attribute every entry to an actor (admin, API key, Telegram bot, user, or system) - Support per-user and global views with event-type and actor filters - Retain history for 90 days with automatic purge; preserve records after user deletion - Pass context through billing and bulk operations to carry actor identity - Add `internal/actor` package for context-scoped action attribution
ActiveDeviceCounts filters connections by last_seen on the hot path (every WorkingUsers read, driven by the access-log tap), but the table's only index was the (user_id, ip) primary key — so the filter was a full scan over a table that grows a row per source IP forever, with no retention sweep. The index has to be covering and explicit. A bare index on last_seen is ignored: SQLite prefers the PK autoindex to satisfy GROUP BY user_id without a sort and scans everything anyway. Leading with last_seen and carrying user_id/ip makes the range both searchable and covering, and INDEXED BY pins it — the planner never runs ANALYZE, so it can't know the device window is only 120s of rows. Also sweep connection rows past ConnectionRetentionDays (30) on the existing retention timer, renamed from eventPurgeLoop since it now covers both tables.
…ness probe The apiLimiter in front of /v1 caps request *rate* but never looks at whether a request authenticated, so on its own it handed a source its full budget of key guesses every minute, forever. Count failures per source IP instead: after 10 invalid keys the address is locked out for 15 minutes, and a valid key clears the record immediately so a misconfigured integration recovers as soon as it's fixed. A request with no credential at all is a bare probe, not a guess, and doesn't spend the budget — otherwise an anonymous prober could lock out an address a real integration shares with it. GET /v1/healthz answers without a key so an uptime monitor doesn't have to carry a credential that can delete users. It returns 503 when Xray is down: the panel may be fine, but the node is carrying no VPN traffic, which is the thing worth being paged about. It lives under the API path rather than at the server root deliberately. A root /healthz answering JSON would fingerprint the panel to any scanner and defeat the decoy; the API segment is also stable across secret rotation, so the monitor URL survives.
Scheduled backups only existed inside the Telegram service, so an operator who never set up a bot had no automatic backups at all — even though internal/backup could already produce them. A local scheduler now writes archives to <dataDir>/backups on the same cron dialect and operator timezone, keeping the newest N. Two ways this could quietly destroy what it exists to protect, both now covered by tests: the backups dir is excluded from the walk (otherwise every archive nests the previous one and grows geometrically), and keep <= 0 means retain everything rather than delete everything, so an unset config can't erase the lot. The walk also skipped hand-rolled recovery copies by ".bak-" only, which let every ".bak.<date>" copy ride along — found on the live box, where seven full DB copies were being carried inside each archive. The cron is validated on save, not at fire time: the scheduler can only log an unparseable expression and skip, which looks exactly like "nothing was due" — so a typo would silently mean no backups until the day someone needed one. The schedule picker is extracted to a shared CronPicker, so the Telegram and local schedules can't drift apart in what they accept.
connguard.Ensure is best-effort and degrades to a no-op when nft is missing or the panel isn't root — it only logs. So an operator who believes the box is protected from connection floods could be running with no guard at all and never find out. The health report now says so. Both checks are live probes (nft list table, reading the sysctl) rather than a remembered boot outcome: "we called Ensure" is not evidence the rules exist. Missing rules while the guard is enabled is a warning; BBR is only informational, since it's a throughput optimization and plenty of healthy kernels don't offer it. connGuardWanted is recorded at boot so the report can tell "off on purpose" (ROSPANEL_CONNLIMIT=off) from "on, but nftables refused it".
… vars internal/datasec said secrets.key was excluded from backups; internal/backup says it is included, and it is — the encrypted DB is useless without it, so a backup that left it out would not restore on a fresh host. The datasec doc and the dead KeyFileName helper were the ones lying, and an operator would have discovered that at the worst possible moment. The consequence is now stated: a backup archive is as sensitive as the key itself. Also: document ROSPANEL_CONNLIMIT* in the README, add the undocumented GET /v1/billing/providers to docs/api.md, note that RoutingOrder includes the opera lane, and mark TLSMode as vestigial (always "acme"; don't branch new behaviour on it) rather than ripping it out of the TLS paths for cosmetics.
staticcheck ST1022: the comment led with "UserEventRetention", dropping the Days suffix, so it didn't match the identifier it documents.
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
1 participant
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.
No description provided.