An empty [[admin]] list leaves the admin API open — the one place ER inverts secure-by-default #54

New issue

Closed

opened 2026-10-26 13:37:27 +00:00 by james.oates · 1 comment

james.oates commented

2026-10-26 13:37:27 +00:00

Owner

Filed because this is a known, deliberate exception that had no issue of its own, so it was only recorded in conversation.

middleware.rs applies the admin host/IP check only when [[admin]] has entries:

if !admin_entries.is_empty() { /* host + allow_ips check */ }

With no [[admin]] block the check is skipped entirely and the admin API — including the dashboard — is reachable from any host that can connect, with API-key auth as the only control. Every other resource in ER treats an empty rule list as deny-all; this is the single inversion of that rule, and CLAUDE.md's "secure by default … an empty rule list is a deny-all, not a no-op" is therefore not true of admin.

It became deliberate with the v1.0.1 dashboard fix, so flipping it is a breaking change for anyone relying on the current behaviour.

Compounding factors:

There is no way to disable the admin API and no separate admin socket, so "loopback only" has to be expressed as an [[admin]] entry with allow_ips = ["127.0.0.1/32", "::1/128"] — opt-in, and not obvious.
The shipped docker-compose.yml sets ALLOWED_ADMIN_HOSTS, which nothing reads (#12), so an operator has a visible-but-inert control that reads like it closes this.

Options, roughly in order of how much they change:

Document it prominently and leave the behaviour — cheapest, keeps the gap.
Default to loopback-only when [[admin]] is absent. Breaking for remote-dashboard users, but matches every other resource and is the behaviour the docs already imply.
Make an explicit [[admin]] block required to enable the admin API at all, so absence means off rather than open.

v2.0.0 is already breaking, so if this is to change, riding along with it is cheaper than a later major. Not doing it unprompted — it is a posture decision, not a defect fix, and it affects anyone currently reaching the dashboard remotely.

Filed because this is a known, deliberate exception that had no issue of its own, so it was only recorded in conversation. `middleware.rs` applies the admin host/IP check only when `[[admin]]` has entries: ```rust if !admin_entries.is_empty() { /* host + allow_ips check */ } ``` With no `[[admin]]` block the check is skipped entirely and the admin API — including the dashboard — is reachable from any host that can connect, with API-key auth as the only control. Every other resource in ER treats an empty rule list as deny-all; this is the single inversion of that rule, and CLAUDE.md's "secure by default … an empty rule list is a deny-all, not a no-op" is therefore not true of admin. It became deliberate with the v1.0.1 dashboard fix, so flipping it is a breaking change for anyone relying on the current behaviour. Compounding factors: - There is no way to disable the admin API and no separate admin socket, so "loopback only" has to be expressed as an `[[admin]]` entry with `allow_ips = ["127.0.0.1/32", "::1/128"]` — opt-in, and not obvious. - The shipped `docker-compose.yml` sets `ALLOWED_ADMIN_HOSTS`, which **nothing reads** (#12), so an operator has a visible-but-inert control that reads like it closes this. Options, roughly in order of how much they change: 1. Document it prominently and leave the behaviour — cheapest, keeps the gap. 2. Default to loopback-only when `[[admin]]` is absent. Breaking for remote-dashboard users, but matches every other resource and is the behaviour the docs already imply. 3. Make an explicit `[[admin]]` block required to enable the admin API at all, so absence means off rather than open. v2.0.0 is already breaking, so if this is to change, riding along with it is cheaper than a later major. Not doing it unprompted — it is a posture decision, not a defect fix, and it affects anyone currently reaching the dashboard remotely.

james.oates referenced this issue from a commit

2026-10-26 14:24:42 +00:00

fix!: refuse ALLOWED_ADMIN_HOSTS and ALLOWED_ADMIN_IPS instead of ignoring them

james.oates referenced this issue

2026-10-26 14:24:56 +00:00

docker-compose.yml sets ALLOWED_ADMIN_HOSTS but no code reads it #12

james.oates referenced this issue

2026-10-26 17:13:15 +00:00

A missing routes config starts silently with zero routes — and an open admin API #66

james.oates commented

2026-10-26 17:15:38 +00:00

Author

Owner

This issue is wrong and I am closing it. Raised by Roku; verified against df35d28.

The premise was that middleware.rs's if !admin_entries.is_empty() skips the host/IP check, leaving the admin API reachable from anywhere with only API-key auth. That reads one middleware in isolation. The actual behaviour:

admin_host_gate (src/middleware.rs:88) wraps all three API routers — admin_routes, service_routes and session_routes (src/main.rs:106). It computes admin_entries.iter().any(|e| e.host == host), and any() over an empty list is false, so with no [[admin]] entries every API request is handed to proxy_handler instead. The API, the dashboard, /health and /ready are simply not served.

That makes the is_empty() branch in admin_ip_middleware unreachable for the no-entries case: the host gate has already refused. With entries present, the middleware finds the matching one and enforces access_allowed, which returns false for an empty rule list — so an [[admin]] entry carrying no [[admin.access]] rule is deny-all, consistent with every other resource rather than an exception to it.

So ER does not invert its secure-by-default rule here. An [[admin]] entry is what enables the API; its access rules are what restrict it; absent either, nothing is served.

Where the claim came from: a note written on 2026-10-20 that was carried forward and repeated — in this issue, in the v2.0.0 release notes, and in two messages to other sessions — without re-reading admin_host_gate. The release notes have been corrected. A memory note asserting the open-admin behaviour has been deleted.

What survives as real, and is tracked elsewhere:

The registration endpoints genuinely do bypass admin_ip_middleware — they require the admin Host and a key but not the via/allow_ips restriction. Rescoped in #55.
/health and /ready being behind the admin Host gate leaves no unauthenticated-and-unrestricted liveness surface for a monitoring probe. Filed as #67.

Closing as not a defect.

**This issue is wrong and I am closing it.** Raised by Roku; verified against `df35d28`. The premise was that `middleware.rs`'s `if !admin_entries.is_empty()` skips the host/IP check, leaving the admin API reachable from anywhere with only API-key auth. That reads one middleware in isolation. The actual behaviour: `admin_host_gate` (`src/middleware.rs:88`) wraps **all three** API routers — `admin_routes`, `service_routes` and `session_routes` (`src/main.rs:106`). It computes `admin_entries.iter().any(|e| e.host == host)`, and `any()` over an empty list is `false`, so with **no** `[[admin]]` entries every API request is handed to `proxy_handler` instead. The API, the dashboard, `/health` and `/ready` are simply not served. That makes the `is_empty()` branch in `admin_ip_middleware` unreachable for the no-entries case: the host gate has already refused. With entries present, the middleware finds the matching one and enforces `access_allowed`, which returns false for an empty rule list — so an `[[admin]]` entry carrying no `[[admin.access]]` rule is **deny-all**, consistent with every other resource rather than an exception to it. So ER does not invert its secure-by-default rule here. An `[[admin]]` entry is what *enables* the API; its access rules are what restrict it; absent either, nothing is served. Where the claim came from: a note written on 2026-10-20 that was carried forward and repeated — in this issue, in the v2.0.0 release notes, and in two messages to other sessions — without re-reading `admin_host_gate`. The release notes have been corrected. A memory note asserting the open-admin behaviour has been deleted. What survives as real, and is tracked elsewhere: - The registration endpoints genuinely do bypass `admin_ip_middleware` — they require the admin Host and a key but not the `via`/`allow_ips` restriction. Rescoped in **#55**. - `/health` and `/ready` being behind the admin Host gate leaves no unauthenticated-and-unrestricted liveness surface for a monitoring probe. Filed as **#67**. Closing as not a defect.

james.oates closed this issue

2026-10-26 17:15:56 +00:00

james.oates referenced this issue

2026-10-26 17:16:06 +00:00

A "scoped service key" has no scope, and the mutating API is the part without the IP gate #55

james.oates referenced this issue

2026-10-26 17:16:42 +00:00

A missing routes config starts silently with zero routes — and an open admin API #66