Issue 116: route-proxy has no authentication or IP-restriction mechanism #109

Merged
jschoubben merged 4 commits from issue/116-route-proxy-has-no-auth-or-ip-restriction into main 2026-09-25 12:28:58 +00:00
Owner

Filed while deciding whether route-proxy can replace HAL's adopted Traefik as the permanent public ingress. Compared against what Traefik is actually doing on this node today, not its general feature set, per the standing rule that the nox mesh must do at minimum what the HAL mesh it replaces already does.

Most of the comparison came out clean or better (streaming uploads, WebSocket upgrades, TLS/ACME safety, multi-hostname routing via the recently-merged contributes many-shape). Two real, confirmed gaps remain, both security-relevant:

  • No authentication mechanism — RedisInsight has no login of its own and depends entirely on Traefik's basicauth middleware today.
  • No IP-restriction mechanism — the gitea-internal route depends on an IP-scoped deny rule.

route-proxy's entire request path is one routing-table lookup then proxy.ServeHTTP — confirmed by reading the whole of main.go. Neither of these routes can move off Traefik until this is closed; migrating them anyway would remove the only access control either currently has.

Full detail, evidence, and open questions on the right shape for the fix are in the report.

Filed while deciding whether `route-proxy` can replace HAL's adopted Traefik as the permanent public ingress. Compared against what Traefik is *actually* doing on this node today, not its general feature set, per the standing rule that the nox mesh must do at minimum what the HAL mesh it replaces already does. Most of the comparison came out clean or better (streaming uploads, WebSocket upgrades, TLS/ACME safety, multi-hostname routing via the recently-merged `contributes` many-shape). Two real, confirmed gaps remain, both security-relevant: - **No authentication mechanism** — RedisInsight has no login of its own and depends entirely on Traefik's `basicauth` middleware today. - **No IP-restriction mechanism** — the gitea-internal route depends on an IP-scoped deny rule. `route-proxy`'s entire request path is one routing-table lookup then `proxy.ServeHTTP` — confirmed by reading the whole of `main.go`. Neither of these routes can move off Traefik until this is closed; migrating them anyway would remove the only access control either currently has. Full detail, evidence, and open questions on the right shape for the fix are in the report.
jschoubben added 1 commit 2026-09-25 09:52:06 +00:00
Comparing route-proxy against what HAL's actual Traefik config does today,
not Traefik's general feature set, per the standing rule that the nox mesh
must do at minimum what the HAL mesh it replaces already does. Everything
else checked out even or better; these two are real, confirmed gaps —
RedisInsight has no login of its own and depends entirely on Traefik's
basicauth middleware, and the gitea-internal route depends on an IP-scoped
deny rule. Neither has any equivalent in route-proxy's single-lookup
request path.
Author
Owner

Review — the conclusion holds, the gap is bigger, and one thing blocks merge

Verified against the code and against the predecessor's whole module catalogue rather than one directory on one node.

The central claim is correct and I confirmed it independently. The request path is handler() → held.find(r.Host) → 404 or proxy.ServeHTTP. No BasicAuth, no Authorization, no RemoteAddr, no X-Forwarded-For, no net.ParseIP, no middleware chain anywhere in the file.

The structural finding the issue is missing

The routing table is map[string]*httputil.ReverseProxy keyed on host alone, and find() strips the port, lowercases, and looks the host up. One host maps to exactly one backend.

So the proxy has no concept of a path — and the incident-response rule this issue depends on is PathPrefix + priority: 100000 shadowing the normal route on the same host. Adding basic auth and an IP list would still not reproduce it. Path-scoped routing with priority is a prerequisite, not a sibling feature, and that ordering should be in the issue because it changes what "fix this" means.

The catalogue survey changes the scope

I surveyed every module for reverse-proxy features. Basic auth has three dependents, not one:

Module What it gates
redis the Redis browser UI — the one the issue names
postgres a database web UI
traefik its own dashboard (twice — also in a desktop flavour)

The database web UI is a worse exposure than the one named, and the issue doesn't mention either of the other two. Four capabilities are missing, not two:

  1. Basic auth — three dependents above.
  2. Outright refusal / source restriction — the incident-response rule.
  3. Path-scoped routing with priority — needed by that rule and by the mail module, which routes an ACME challenge path on a shared host.
  4. Redirect rules — two live routes canonicalise www to apex via redirectRegex. Not in the catalogue, so a catalogue-only survey misses them; they fail silently rather than erroring, which makes them the easiest to lose.

What compared cleanly, confirmed: request-body buffering is used by four modules (not just the object store) and httputil.ReverseProxy covers all of them; the host policy really does refuse to certify unrouted names; staging really is the ACME default via issuer().

Evidence corrections

  • The stated method doesn't match the evidence. The report says it read /services/traefik/dynamic/ directly. The Redis UI's basic auth is not there — it is a container label. Reading that directory would not have found it. Four files there declare middleware, and two are www→apex redirects the report doesn't mention.
  • main.go is 360 lines, not 261. The last change to it landed the day before this was filed, so "confirmed by reading the whole of main.go (261 lines)" doesn't hold as written. The conclusion survives — I re-derived it — but the basis as stated does not.
  • "An IP-scoped deny rule" mischaracterises it. It is an allow-list whose single entry is an RFC 5737 documentation address, i.e. deny-everyone; the middleware is literally named deny-all. Describing it as IP-scoped implies the fix is an allow-list of real addresses, when what is needed is the ability to refuse on a path.
  • Severity is understated. That file's own header records it as incident response to an actual compromise, mitigating a known-exploited write primitive. "A real exposure, not a cosmetic gap" undersells a live mitigation. The same header also already records "durable home is the route-proxy module; re-home when convenient" — useful corroboration that this isn't a new idea.
  • Minor: the contributes many-shape is cited as PR #55; the merge I can see in mesh-controller is #57. Worth checking.

⚠️ Blocks merge — disclosure

AGENTS.md: "Nothing here may contain routable addresses, real domain names … absolute paths." This repository is public.

  • line 23 contains a real hostname
  • line 19 contains an absolute path on the node

No other merged issue report in 04-ISSUES/ contains a real hostname — I checked. Use role names and generic paths, as the rest of the folder does.

On the open questions

One consideration worth adding: an htpasswd hash carried in a route contribution puts a credential in a declaration. That interacts with how the mesh handles secrets, and it is an ADR-shaped question rather than an implementation detail — worth settling before either option in the report is chosen.

status: located and located-in: both look right — the latter matches 08-connectivity's code:. Though since the fix may be a contract change rather than a change to a reference implementation, the owner may widen once the shape is decided.

## Review — the conclusion holds, the gap is bigger, and one thing blocks merge Verified against the code and against the predecessor's whole module catalogue rather than one directory on one node. **The central claim is correct and I confirmed it independently.** The request path is `handler()` → `held.find(r.Host)` → 404 or `proxy.ServeHTTP`. No `BasicAuth`, no `Authorization`, no `RemoteAddr`, no `X-Forwarded-For`, no `net.ParseIP`, no middleware chain anywhere in the file. ### The structural finding the issue is missing The routing table is `map[string]*httputil.ReverseProxy` **keyed on host alone**, and `find()` strips the port, lowercases, and looks the host up. **One host maps to exactly one backend.** So the proxy has no concept of a path — and the incident-response rule this issue depends on is `PathPrefix` + `priority: 100000` shadowing the normal route *on the same host*. **Adding basic auth and an IP list would still not reproduce it.** Path-scoped routing with priority is a prerequisite, not a sibling feature, and that ordering should be in the issue because it changes what "fix this" means. ### The catalogue survey changes the scope I surveyed every module for reverse-proxy features. **Basic auth has three dependents, not one:** | Module | What it gates | |---|---| | `redis` | the Redis browser UI — the one the issue names | | `postgres` | a **database web UI** | | `traefik` | **its own dashboard** (twice — also in a desktop flavour) | The database web UI is a worse exposure than the one named, and the issue doesn't mention either of the other two. **Four capabilities are missing, not two:** 1. **Basic auth** — three dependents above. 2. **Outright refusal / source restriction** — the incident-response rule. 3. **Path-scoped routing with priority** — needed by that rule *and* by the mail module, which routes an ACME challenge path on a shared host. 4. **Redirect rules** — two live routes canonicalise `www` to apex via `redirectRegex`. Not in the catalogue, so a catalogue-only survey misses them; they fail *silently* rather than erroring, which makes them the easiest to lose. What compared cleanly, confirmed: request-body buffering is used by four modules (not just the object store) and `httputil.ReverseProxy` covers all of them; the host policy really does refuse to certify unrouted names; staging really is the ACME default via `issuer()`. ### Evidence corrections - **The stated method doesn't match the evidence.** The report says it read `/services/traefik/dynamic/` directly. The Redis UI's basic auth **is not there** — it is a container label. Reading that directory would not have found it. Four files there declare middleware, and two are `www`→apex redirects the report doesn't mention. - **`main.go` is 360 lines, not 261.** The last change to it landed the day before this was filed, so "confirmed by reading the whole of `main.go` (261 lines)" doesn't hold as written. The conclusion survives — I re-derived it — but the basis as stated does not. - **"An IP-scoped deny rule" mischaracterises it.** It is an allow-list whose single entry is an RFC 5737 documentation address, i.e. deny-everyone; the middleware is literally named `deny-all`. Describing it as IP-scoped implies the fix is an allow-list of real addresses, when what is needed is the ability to refuse on a path. - **Severity is understated.** That file's own header records it as incident response to an actual compromise, mitigating a known-exploited write primitive. "A real exposure, not a cosmetic gap" undersells a live mitigation. The same header also already records *"durable home is the route-proxy module; re-home when convenient"* — useful corroboration that this isn't a new idea. - Minor: the `contributes` many-shape is cited as **PR #55**; the merge I can see in `mesh-controller` is **#57**. Worth checking. ### ⚠️ Blocks merge — disclosure `AGENTS.md`: *"Nothing here may contain routable addresses, real domain names … absolute paths."* This repository is public. - **line 23** contains a real hostname - **line 19** contains an absolute path on the node No other merged issue report in `04-ISSUES/` contains a real hostname — I checked. Use role names and generic paths, as the rest of the folder does. ### On the open questions One consideration worth adding: an htpasswd hash carried in a route contribution puts **a credential in a declaration**. That interacts with how the mesh handles secrets, and it is an ADR-shaped question rather than an implementation detail — worth settling before either option in the report is chosen. `status: located` and `located-in:` both look right — the latter matches `08-connectivity`'s `code:`. Though since the fix may be a contract change rather than a change to a reference implementation, the owner may widen once the shape is decided.
jschoubben added 1 commit 2026-09-25 11:24:37 +00:00
Two things the report got wrong, and one it could not have found the way it looked.

Disclosure first: it carried a real hostname and an absolute node path, in a public
repository. Both are gone; the ingress, the modules and the routes are named by role, as the
rest of 04-ISSUES does.

The count was low. Basic authentication has three dependents in the catalogue, not one — the
key-value store's browser UI, a database web UI, and the ingress's own dashboard. All three
are credential-less admin surfaces whose only gate is a middleware the mesh's proxy lacks.
The earlier version read only the node's dynamic configuration directory, which cannot see
what modules declare as container labels; counting needs both sources, and the report now
says so.

Two gaps were missing entirely. Redirect rules: two live routes canonicalise a www name onto
its apex, they exist only on the node and not in the catalogue, and they fail silently rather
than erroring. And path-scoped routing with priority, which is the one that reorders the
issue: the table maps host to exactly one target, so a host cannot be routed two ways, and
the refusal rule matches a path on a host already routed elsewhere. Authentication and a
source filter would not make it expressible. Path scoping is a prerequisite, not a sibling.

Also corrected: the refusal rule was described as an address-scoped deny. It is an allow-list
holding a single documentation-range address — deny-everyone — so reading it as address-scoped
points at the wrong fix. And its severity was understated: its own header records it as
incident response closing an abused write primitive, which is not "a real exposure" but a live
mitigation.

The open questions now say plainly that they are design questions and the fix should not be
written before they are answered, and one is added: whether a declaration may carry a
credential at all.
jschoubben added 1 commit 2026-09-25 11:48:20 +00:00
Issue 116 found the mesh's proxy applies nothing to a request — host lookup, forward. Against
what the replaced ingress actually relies on, four capabilities are missing: authentication
(three dependents, each gating an admin surface with no login of its own), refusal scoped to a
path (one, a live incident mitigation), path-scoped routing with priority, and redirect.

Policy goes on the route rather than beside it. A proxy-side settings layer keyed by route name
would keep the grant literally clean, but then "what protects this route" is answered from two
files nothing keeps in step — and a route's protection is part of what a route is.

The set is closed at those four, so a fifth is an amendment and each addition is earned by a
dependent that exists. An open middleware surface was rejected: it recreates what is being
replaced, and narrowing one later is far harder than widening a closed one.

Where policy needs a credential the declaration names a secret and never carries the value,
which keeps the existing secret machinery the only thing holding credentials. Inlining a hash
was rejected as the first credential in a declaration — a precedent easier to set than withdraw.

This re-keys the routing table by host and path with priority, which follows from the decision
rather than being a separate one: two of the four need one host routed more than one way. Equal
priorities must resolve identically every time or the proxy stops being reproducible.

The record says how it is checked, including the negative case that rots quietly — a
declaration carrying a credential value rather than a reference must be refused, so the
rejected option cannot return by accident.

08-connectivity §3 names the record and gains the subsection; issue 116 gains amended-design.
jschoubben added 1 commit 2026-09-25 12:25:28 +00:00
The gap is closed in the proxy: policy applies, the four capabilities exist, the table is keyed
by host and path with a total ordering, and the two failure modes that rot quietly are held by
tests — a declaration carrying a credential refused rather than served, an unreadable secret
failing closed.

Resolved rather than left open because the issue reports a gap in the proxy and that gap is
gone. But the record says plainly what it does not yet allow: an operator still cannot move the
affected routes, because that needs the mesh side — a manifest able to declare these values and
the controller minting the secret auth names. Until both exist the capability is reachable only
by writing the routes file by hand. That is the ordinary build-out of a contract this issue's
decision created, and it belongs to to-be 08 rather than here.

The open questions are marked answered and kept rather than deleted, pointing at ADR 0108 —
what was rejected and why is the half worth having, and a section still saying "the fix should
not be written before these are answered" after the fix was written reads as though nobody
looked.

One finding kept in the record: priority was read with the reader for ports, which caps at
65535, and the one real rule this reproduces is declared at 100000. It parsed to zero, so
refusal and path scoping would have shipped looking complete and doing nothing on the only case
that motivated them. A validator borrowed from a neighbouring field is a silent default.
Author
Owner

Review, and resolved — 367df38

Reviewed in an isolated worktree against current main. cycle 267 documents / records pass / index current. Links resolve, disclosure scan clean.

Now carries the whole cycle

Issue 116 amended — disclosure scrubbed, count corrected, two missing gaps added, now status: resolved
ADR 0108 a route carries the policy applied to a request
08-connectivity §3 names the record, updated: bumped
Fix mesh-controller PR #58, merged

Two things the review changed

resolved was overclaiming, so the record now says what it does not allow. The gap in the proxy is closed, but an operator still cannot move the affected routes — that needs the mesh side, a manifest able to declare these values and the controller minting the secret auth names. Until then the capability is reachable only by writing the routes file by hand. Left as resolved because the issue reports a gap in the proxy and that gap is gone; the rest is ordinary build-out of a contract this decision created, and belongs to to-be 08.

The open questions said the fix should not be written before they were answered — after it had been written. They're now marked answered, pointing at ADR 0108, and kept rather than deleted: what was rejected and why is the half worth having.

A real bug the code review caught, recorded here because the lesson generalises

Priority was read with the reader for ports, which caps at 65535 — and the one real rule this exists to reproduce is declared at 100000. It parsed to zero, so refusal and path scoping would both have shipped looking complete, passing their own tests, and doing nothing on the only case that motivated them.

Fixed in e11e137 with a regression test that goes through the proxy, because at the parser the value looked fine. A validator borrowed from a neighbouring field is a silent default — that's in the report.

Caveat on this review

I authored all of it, so this is weaker than an independent read. The issue text in particular started as someone else's and I rewrote it substantially. The factual claims are re-derived from the code and the catalogue rather than from the earlier draft, but the framing choices are mine and worth a second opinion.

Merging.

## Review, and resolved — `367df38` Reviewed in an isolated worktree against current `main`. `cycle` 267 documents / `records` pass / `index` current. Links resolve, disclosure scan clean. ### Now carries the whole cycle | | | |---|---| | Issue 116 | amended — disclosure scrubbed, count corrected, two missing gaps added, now **`status: resolved`** | | ADR 0108 | a route carries the policy applied to a request | | `08-connectivity` §3 | names the record, `updated:` bumped | | Fix | **mesh-controller PR #58**, merged | ### Two things the review changed **`resolved` was overclaiming, so the record now says what it does not allow.** The gap *in the proxy* is closed, but an operator still cannot move the affected routes — that needs the mesh side, a manifest able to declare these values and the controller minting the secret `auth` names. Until then the capability is reachable only by writing the routes file by hand. Left as `resolved` because the issue reports a gap in the proxy and that gap is gone; the rest is ordinary build-out of a contract this decision created, and belongs to to-be 08. **The open questions said the fix should not be written before they were answered — after it had been written.** They're now marked answered, pointing at ADR 0108, and kept rather than deleted: what was rejected and why is the half worth having. ### A real bug the code review caught, recorded here because the lesson generalises Priority was read with the reader for **ports**, which caps at 65535 — and the one real rule this exists to reproduce is declared at **100000**. It parsed to zero, so refusal and path scoping would both have shipped *looking complete, passing their own tests, and doing nothing on the only case that motivated them.* Fixed in `e11e137` with a regression test that goes through the proxy, because at the parser the value looked fine. **A validator borrowed from a neighbouring field is a silent default** — that's in the report. ### Caveat on this review I authored all of it, so this is weaker than an independent read. The issue text in particular started as someone else's and I rewrote it substantially. The factual claims are re-derived from the code and the catalogue rather than from the earlier draft, but the framing choices are mine and worth a second opinion. Merging.
jschoubben merged commit 56669ee23b into main 2026-09-25 12:28:58 +00:00
jschoubben deleted branch issue/116-route-proxy-has-no-auth-or-ip-restriction 2026-09-25 12:29:05 +00:00
Sign in to join this conversation.
No Reviewers
No labels
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: novox/hq#109