fix(security): require authentication on /metrics endpoint

[agh]

Alternative approach to #299 that addresses the security concern without disabling the endpoint.

Problem

/metrics is the only endpoint on the device with no authentication. Every other route is either public-by-design (/device/status, /auth/login-local) or behind protectedMiddleware (cookie auth) or basicAuthProtectedMiddleware (basic auth). The metrics endpoint sits outside all route groups with no auth at all.

Why #299 stalled

PR #299 proposed a config toggle to disable metrics entirely. This drew pushback from @SuperQ who correctly pointed out that disabling metrics does not improve security — the endpoint should simply require the same auth as everything else. @IDisposable raised a CPU overhead concern, but gathering only happens on scrape so a toggle does not help there either. The PR has been in draft with needs-rework since March 2025.

What this PR does instead

Add a metricsAuthMiddleware that accepts authentication via two methods:

1. Session cookie (authToken) — same as protectedMiddleware. Browser access to /metrics works if logged in.

2. HTTP basic auth — same password verification as basicAuthProtectedMiddleware. This is how Prometheus scrape configs authenticate:

   scrape_configs:
     - job_name: jetkvm
       basic_auth:
         username: admin  # username is ignored, only password is checked
         password: <device-password>
       static_configs:
         - targets: ['192.168.1.100']

3. No-password mode passthrough — when localAuthMode is noPassword, the middleware passes all requests through. Consistent with every other protected endpoint on the device.

What this does NOT do

• No config toggle. The endpoint is always available. Monitoring setups do not break.
• No new config fields. No UI changes. No migration needed.
• No change for no-password mode users. Their scrape configs keep working as-is.

How it addresses each concern from #299

Concern	#299 approach	This PR
@ym: metrics endpoint is unauthenticated	Disable by default	Require auth (cookie or basic auth)
@SuperQ: do not disable metrics	Disabled by default	Always available, just authenticated
@IDisposable: CPU overhead	Config toggle	Not relevant — gathering only on scrape regardless

[SuperQ]

LGTM, thanks! I haven't looked at this code in a while to see how the config/auth is handled.

Sorry if this is obvious but is there a way or need to get the TLS status before enabling auth?

I worry about people enabling auth without TLS first. It's going to encourage users to leak their credentials over the wire without knowing that basic auth / token isn't protected.

[agh]

Good point — worth thinking about. This PR doesn't change the exposure surface though: the cookie auth (authToken) is already sent over plaintext HTTP with Secure: false, and the developer mode routes use basic auth over HTTP too. The metrics basic auth here sits alongside those existing paths.

You're right that it would be nice to nudge users toward TLS before relying on auth. Some pieces are already in the codebase that could help with that:

• config.TLSMode tracks whether TLS is enabled ("", "self-signed", "custom")
• getTLSState() in web_tls.go exposes the current state
• Issue #1092 is specifically about adding HTTP→HTTPS redirect when TLS is enabled, which would be the natural place to tackle credential protection on the wire

Longer term, setting Secure: true on the auth cookie when TLS is active and landing the HTTPS redirect from #1092 would cover this properly across all endpoints — not just metrics. Both feel like good follow-ups.

[baffles]

You admit that "disabling metrics does not improve security" but by forcing authentication anyway, now I need to put my password in plain text in a configuration file just to scrape statistics, which actively harms security by adding a new attack surface and potential place for that credential to be leaked.

I think this should be configurable, or there should be the ability to configure an alternate auth token for this that is read-only.

[agh]

That quote is about PR #299's approach of adding a toggle to disable /metrics entirely — turning off the endpoint doesn't make it more secure, it just removes observability. Authentication is a different matter; an unauthenticated endpoint that exposes Go runtime internals, system uptime, request counts, and connection state is a genuine information disclosure vector.

On the credential storage concern — that's a fair point, though it applies to any authenticated Prometheus target, not specifically to this change. Prometheus's own documentation recommends basic auth for scrape targets and supports basic_auth_password_file to avoid inline plaintext. The device password already lives in scrape configs for anyone hitting the authenticated API today.

A dedicated read-only metrics token is a reasonable idea and would address the concern about reusing the admin credential. That's a larger feature though (new config field, UI, token generation/storage), and I don't think it should gate closing the unauthenticated hole. Happy to open an issue to track it as a follow-up if that sounds useful.

[baffles]

I think the main concern is the violation of the principle of least privilege due to the minimalist authentication system in place. The authentication itself isn't the problem, but the fact that you have to give it a full-access credential is the concern. I'm on the fence on which is worse, but opening an issue to add such a low-privilege token does at least start to address my concern!

[agh]

I'm going to wait for @adamshiervani to weigh in. It's ultimately down to what he's prepared to merge.