Repository navigation
feat(auth)!: secure defaults for authentication, notifications, request limits, and reports - #971
Merged
Merged
Conversation
…ig dir pkg/secrets.EnsureSecret returns the secret stored in <dir>/<name>, creating a random 64 character hex value with mode 0600 when the file is missing or empty. Used for the generated API token and JWT secret.
web.auth.enabled now defaults to true. - Omnibus (collectors installed next to the server): when web.auth.token is empty the server generates a token at first start, saves it to <web.secrets.dir>/.auth-token with mode 0600, and logs the file location (never the value). The collector config reads that file as the lowest precedence api.token default, so in-container collectors authenticate without configuration. - Hub/spoke: the server refuses to start with an error naming web.auth.token unless web.auth.enabled is explicitly false. - web.auth.enabled=false stays available and logs a warning at start. - A missing web.auth.jwt_secret is generated once and saved to <web.secrets.dir>/.jwt-secret so sessions survive restarts; an unwritable directory falls back to a per-process secret with a warning. - The "token required when auth enabled" check moves from config validation to server startup, where the omnibus case can generate one. - Remove the hard errors for the long-deprecated notify.filter_attributes and notify.level keys; they are now ignored. - Add config keys web.secrets.dir, web.listen.trusted_proxies, web.listen.max_body_bytes and web.reports.path (used by the following commits). BREAKING CHANGE: API authentication is enabled by default. Hub/spoke web containers exit at startup unless SCRUTINY_WEB_AUTH_TOKEN (web.auth.token) is set, and every collector must send the same value as COLLECTOR_API_TOKEN. The omnibus image generates a token at first start and saves it to /opt/scrutiny/config/.auth-token; the config volume must be writable. Set SCRUTINY_WEB_AUTH_ENABLED=false to keep running without authentication, or stay on the :1 image tag. See docs/UPGRADE_2.0.md. BREAKING CHANGE: notify.filter_attributes and notify.level no longer fail config validation. They are silently ignored; remove them from scrutiny.yaml.
- Add web.listen.max_body_bytes (default 16 MiB). A request declaring a larger Content-Length gets HTTP 413 before any handler runs; bodies without a declared length are wrapped in http.MaxBytesReader. A smartctl report is tens of KiB and a FARM-merged report a few hundred KiB. - Add web.listen.trusted_proxies (default none) and call SetTrustedProxies, so X-Forwarded-For is honoured only from listed proxies. Invalid entries stop startup with a config error. - Prune expired login rate limiter records for every IP on each recorded failure so the map cannot grow without bound. BREAKING CHANGE: the server no longer trusts X-Forwarded-For from any client. Behind a reverse proxy, set web.listen.trusted_proxies (SCRUTINY_WEB_LISTEN_TRUSTED_PROXIES, space-separated) to the proxy address or CIDR; until then logs and the login rate limiter see the proxy IP. Requests larger than 16 MiB are rejected unless web.listen.max_body_bytes is raised.
script:// targets execute a program on the host. Previously any UI or API caller could store one and trigger it with the per-URL test endpoint. - POST /api/settings/notify-urls returns 400 for script:// URLs (case and whitespace insensitive). - POST /api/settings/notify-urls/:id/test refuses a stored script:// URL. - LoadDatabaseUrls and LoadHeartbeatDatabaseUrls skip database-sourced script:// URLs with a warning, and Send drops them from the merged list, so rows stored by older releases and queued outbox entries never execute. The rows stay in the database. - script:// in notify.urls (config file or SCRUTINY_NOTIFY_URLS) keeps working. BREAKING CHANGE: script:// notification URLs can no longer be added through the UI or API, and ones already stored in the database are skipped with a warning instead of running. Move them to notify.urls in scrutiny.yaml and delete the stored entry.
The report_pdf_path dashboard setting let an API caller choose where the server writes PDFs and which directory the history endpoint lists. - Remove report_pdf_path from the settings model. The scheduler writes to web.reports.path (default /opt/scrutiny/reports). - Log one warning when a stored report_pdf_path differs from web.reports.path. - GET /api/reports/history reads web.reports.path, no longer returns the server filesystem path, and no longer echoes filesystem error text. - Update docs/openapi.yaml for both changes. BREAKING CHANGE: the report_pdf_path setting is removed from the settings API and ignored if stored. Set web.reports.path (SCRUTINY_WEB_REPORTS_PATH) in the config file to change the PDF directory. GET /api/reports/history no longer returns a path field.
Update AUTH.md, UPGRADE_2.0.md, example.scrutiny.yaml and the README config tables for default-on authentication, generated secrets, trusted proxies, the request body limit, config-only script:// URLs and web.reports.path.
Starosdev
added
security
Pass -- before the apprise target so it is never parsed as a flag, reject apprise+ targets starting with - on save and the test endpoint, skip them when loaded from the database, and return a generic error from the per-URL test endpoint (details stay in the server log) so credentials and apprise output are not echoed to the caller.
…ETS_DIR in collectors EnsureSecret writes a temp file and publishes it with a hard link so concurrent callers converge on one value, re-reads the file, and refuses symlinked or non-regular targets. The collector reads the generated token from SCRUTINY_WEB_SECRETS_DIR when set, matching the web server.
… auth docs Limit /api/auth/login bodies to 64 KiB, log a one-time warning when X-Forwarded-For arrives from a peer outside web.listen.trusted_proxies, and document proxy lockout risk, JWT secret rotation, and HEAD /api/health.
Merged
12 of 14 tasks
…symlinks EnsureSecret writes a 0600 temp file, syncs it, and publishes it with a hard link so concurrent callers converge on one value. It falls back to an O_EXCL create on filesystems without hard links, refuses symlinked or non-regular targets, re-reads the file after writing, and always removes the temp file.
…arded-For Requests to /api/auth/* get a 64 KiB body cap in addition to the global limit. ForwardedForWarningMiddleware logs one warning, naming only the peer IP, when a request carries X-Forwarded-For from a peer outside web.listen.trusted_proxies. The header is never trusted by it.
…vailable Replace the O_EXCL-then-write fallback with a <name>.lock exclusive lock (stale after 30s): under the lock the target is re-checked and a synced temp file is renamed over it, so readers never see an empty or partial secret. An empty leftover is replaced only under the lock, and EnsureSecret retries or errors instead of returning an empty value.
3 of 14 tasks
32 of 40 tasks
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
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.
Summary
WP-B (security defaults) for the 2.0.0 release. API authentication is on by default,
script://notification URLs are config-only, request bodies are size-limited, client IPs fromX-Forwarded-Forare trusted only from configured proxies, and the PDF report directory is config-only. Adds a shared helper that generates and persists secrets in the config directory, which the InfluxDB credential work reuses.Type of Change
Related Issues
Related to #965
Changes Made
pkg/secrets:EnsureSecret(dir, name) (secret, created, err)creates a 64-hex-character secret with mode 0600 (directory 0700), reuses an existing one, and replaces an empty file;Read(dir, name).web.auth.enableddefaults totrue(breaking). Startup (auth.EnsureStartupAuth, first step ofserver.Start):/opt/scrutiny/config/.auth-tokenif none is configured; the local collector reads it as its lowest-precedenceapi.tokendefault, so cron, one-shot, and API-triggered runs work without extra config. The token value is never logged.web.auth.token/SCRUTINY_WEB_AUTH_TOKEN. Collectors need the same value inCOLLECTOR_API_TOKEN.web.auth.enabled: falseremains an explicit opt-out and logs a warning./api/healthstays public..jwt-secretso sessions survive restarts; an unwritable config dir falls back to a per-process secret with a warning.web.listen.max_body_bytes(default 16 MiB) return 413 (breaking for oversized clients).X-Forwarded-Foris no longer trusted by default; setweb.listen.trusted_proxiesbehind a reverse proxy (breaking for proxied rate limiting and logging). Login rate-limiter entries are pruned.script://notification URLs are rejected on the UI/API path; existing database rows withscript://are skipped with a warning (rows kept), andnotify.Sendalso drops database-sourced script URLs. Script URLs remain allowed innotify.urlsin the config file (breaking).report_pdf_pathdashboard setting is removed (useweb.reports.path);/api/reports/historyno longer returns absolute paths. A stored 1.x value logs a one-time warning (breaking).notify.filter_attributes/notify.levelno longer error; they are ignored.docs/AUTH.md,docs/UPGRADE_2.0.md(auth, script URLs, report path sections),README.mdconfig tables,docs/openapi.yaml,example.scrutiny.yaml.Testing
go build ./...,go vet ./...,go test ./...(40 packages) pass;GOOS=linux golangci-lint run --new-from-rev=origin/beta ./...(v2.14.0) 0 issues. New tests: config defaults and removed keys; startup auth (hub/spoke failure, omnibus generation with 0600 and no token in logs, disabled warning, unwritable dir); 413 body limit; trusted proxies; script URL rejection on API and skip on load and send; reports history without paths; rate-limiter pruning; PDF directory source;pkg/secrets.Checklist
Notes
BREAKING CHANGE:footer.docker exec <container> cat /opt/scrutiny/config/.auth-token) on first load; removereport_pdf_pathfromapp.config.tsanddashboard-settings.component.ts(the backend ignores the unknown field, so saves keep working); surface the 400 when saving ascript://URL./opt/scrutiny/configmust stay writable.betapublishes the:betaimages. Omnibus deployments keep collecting with the generated token, and the web UI then requires that token. Hub/spoke deployments needSCRUTINY_WEB_AUTH_TOKENandCOLLECTOR_API_TOKENbefore upgrading.Review: required (alters a security and access boundary)