Docs advertise secret-based AUTH + per-DB ACLs, but auth was intentionally removed — update docs (and decide if it returns) #45

New issue

Open

opened 2026-06-10 14:30:21 +00:00 by sameh-farouk · 1 comment

sameh-farouk commented

2026-06-10 14:30:21 +00:00

Member

The README/PURPOSE still document an authentication / authorization feature that no longer exists in the code. This was an intentional removal, so the actionable bug here is the stale documentation; whether auth should come back is a separate product decision (noted at the end).

Docs still claim it

README.md:3 — "…secret-based authentication."
README.md:34 — "Secret-based Auth — AUTH <secret> / SAUTH <secret> with per-user ACLs"
README.md:36 — "Multi-Database with ACL — Per-database Read/Write/Admin permissions per user"
README.md:98 — a whole "Start Server (With Authentication)" section (--admin-secret), plus :118 SAUTH, :232/:244
PURPOSE.md:9 — "Supports secret-based authentication with per-user, per-database ACLs (Read / Write / Admin)"

Code no longer has it

crates/hero_db/src/auth.rs: "Authentication and authorization have been removed from hero_db — every connection has full access." SessionContext now carries only current_db.
crates/hero_db/src/server.rs:7: "There is no authentication or authorization — every connection has full access."
AUTH / SAUTH are not in the dispatch — clients get ERR unknown command 'AUTH'.

This was deliberate (not a regression)

Auth was specified and implemented under #18 (an auth.* handler module with Read/Write/Admin ACLs; "Default is no auth (preserved)").
It was then intentionally removed in commit 627a41a (2026-05-31) — a decompose/cleanup refactor that deleted the hero_db_app Dioxus crate and the "related admin/auth server handlers" together. The prior auth.rs held a full secret-based per-DB ACL model.
Plausible rationale (consistent with the Hero architecture): TCP ingress is fronted by hero_router / hero_proxy, which handle TLS/OAuth, so per-service AUTH may have been deliberately delegated to the mesh layer rather than duplicated in hero_db.

Actionable fix (docs)

Update README/PURPOSE to reflect reality: hero_db runs open — every connection has full access, and access control is expected at the socket/host/mesh layer (router/proxy), not in-process. Remove the AUTH/SAUTH/--admin-secret/per-DB-ACL sections (or move them to a "removed / see mesh-level auth" note). This can fold into the doc-accuracy work in #44.

Open product question (not a bug)

hero_db binds 0.0.0.0:6378 by default. If it's ever exposed beyond loopback/Unix sockets without a proxy in front, "open mode" is a real exposure. Decision for the maintainers: (a) document open-mode + recommend loopback/mesh-only binding, or (b) restore the per-DB auth model from #18. Flagging for a maintainer call — defaulting to (a).

Correction to my earlier framing of this issue: the auth removal was intentional (commit 627a41a), so this is scoped to docs + a product decision, not a "missing feature" defect. Related: #18 (original auth spec), #44 (doc accuracy).

The README/PURPOSE still document an **authentication / authorization** feature that no longer exists in the code. This was an **intentional removal**, so the actionable bug here is the **stale documentation**; whether auth should come back is a separate product decision (noted at the end). ## Docs still claim it - `README.md:3` — "…secret-based authentication." - `README.md:34` — "**Secret-based Auth** — `AUTH <secret>` / `SAUTH <secret>` with per-user ACLs" - `README.md:36` — "**Multi-Database with ACL** — Per-database Read/Write/Admin permissions per user" - `README.md:98` — a whole "Start Server (With Authentication)" section (`--admin-secret`), plus `:118` `SAUTH`, `:232`/`:244` - `PURPOSE.md:9` — "Supports secret-based authentication with per-user, per-database ACLs (Read / Write / Admin)" ## Code no longer has it - `crates/hero_db/src/auth.rs`: *"Authentication and authorization have been removed from hero_db — every connection has full access."* `SessionContext` now carries only `current_db`. - `crates/hero_db/src/server.rs:7`: *"There is no authentication or authorization — every connection has full access."* - `AUTH` / `SAUTH` are not in the dispatch — clients get `ERR unknown command 'AUTH'`. ## This was deliberate (not a regression) - Auth was specified and implemented under #18 (an `auth.*` handler module with Read/Write/Admin ACLs; "Default is no auth (preserved)"). - It was then **intentionally removed in commit `627a41a` (2026-05-31)** — a decompose/cleanup refactor that deleted the `hero_db_app` Dioxus crate and the "related admin/auth server handlers" together. The prior `auth.rs` held a full secret-based per-DB ACL model. - Plausible rationale (consistent with the Hero architecture): TCP ingress is fronted by `hero_router` / `hero_proxy`, which handle TLS/OAuth, so per-service AUTH may have been deliberately delegated to the mesh layer rather than duplicated in hero_db. ## Actionable fix (docs) Update README/PURPOSE to reflect reality: hero_db runs **open — every connection has full access**, and access control is expected at the socket/host/mesh layer (router/proxy), not in-process. Remove the `AUTH`/`SAUTH`/`--admin-secret`/per-DB-ACL sections (or move them to a "removed / see mesh-level auth" note). This can fold into the doc-accuracy work in #44. ## Open product question (not a bug) hero_db binds `0.0.0.0:6378` by default. If it's ever exposed beyond loopback/Unix sockets **without** a proxy in front, "open mode" is a real exposure. Decision for the maintainers: (a) document open-mode + recommend loopback/mesh-only binding, or (b) restore the per-DB auth model from #18. Flagging for a maintainer call — defaulting to (a). --- *Correction to my earlier framing of this issue: the auth removal was intentional (commit `627a41a`), so this is scoped to docs + a product decision, not a "missing feature" defect. Related: #18 (original auth spec), #44 (doc accuracy).*

sameh-farouk changed title from ~~Advertised auth/ACL feature is absent: docs promise secret-based AUTH + per-DB ACLs, but auth was removed from the server~~ to Docs advertise secret-based AUTH + per-DB ACLs, but auth was intentionally removed — update docs (and decide if it returns)

2026-06-10 14:44:29 +00:00

sameh-farouk commented

2026-06-10 14:44:32 +00:00

Author

Member

Reframed after checking history: auth was intentionally removed in 627a41a (2026-05-31), bundled with deleting the hero_db_app Dioxus crate — not a regression. Rescoped this issue to the stale docs (the real bug) plus an explicit product decision on whether auth returns. See #18 for the original auth spec.

Reframed after checking history: auth was **intentionally** removed in `627a41a` (2026-05-31), bundled with deleting the `hero_db_app` Dioxus crate — not a regression. Rescoped this issue to the stale docs (the real bug) plus an explicit product decision on whether auth returns. See #18 for the original auth spec.