feat(api): optional house password on the LAN - #914
Conversation
Off by default. When on, protected LAN routes need a Bearer secret; loopback and an app session do not. Live status stays readable as a viewer.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d322e529c5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| type API struct { | ||
| Port int `yaml:"port" json:"port"` | ||
| Port int `yaml:"port" json:"port"` | ||
| LANAuth bool `yaml:"lan_auth,omitempty" json:"lan_auth,omitempty"` |
There was a problem hiding this comment.
Because this field is accepted by POST /api/config and hot reload, setting lan_auth: true bypasses handleAuthPassword. On a fresh installation there is no lan_auth_password row, so every verification fails and /api/auth/password is itself protected, leaving all non-loopback LAN administration locked out until the operator can use loopback or the phone app. Generic config changes can also disable auth without clearing the hash; reject independent transitions or route them through the credential lifecycle.
Useful? React with 👍 / 👎.
| if !lanAuthOn(policy) || isLoopbackClient(r.RemoteAddr) || !isLocalClient(r.RemoteAddr) { | ||
| return false, r | ||
| } | ||
| secret, ok := parseBearer(r.Header.Get("Authorization")) | ||
| if !ok { | ||
| return false, r | ||
| } | ||
| houseOK := admitLANSecret(policy.VerifyLANSecret, secret) |
There was a problem hiding this comment.
When a protected request uses a public/FQDN host through a private-address client or reverse proxy, this check treats its API Bearer token as a house-password attempt before remoteRequired is calculated. The request can subsequently pass remote-token authentication, but each successful request still counts as a failed LAN guess, so five legitimate API calls globally lock out the correct house password for 30 seconds. Determine whether remote-token authentication applies before calling admitLANSecret.
Useful? React with 👍 / 👎.
| s.deps.Cfg.API.LANAuth = enabled | ||
| cfgCopy := *s.deps.Cfg | ||
| s.deps.CfgMu.Unlock() | ||
| if err := s.deps.SaveConfig(s.deps.ConfigPath, &cfgCopy); err != nil { |
There was a problem hiding this comment.
If SaveConfig fails, such as with a read-only config mount or disk error, the shared configuration has already been changed even though the endpoint returns 500. A failed disable therefore drops LAN protection for the running process, while a failed enable immediately locks protected routes and may leave the newly stored hash orphaned from the on-disk setting. Save a candidate configuration before publishing it, or roll the in-memory value back on failure.
Useful? React with 👍 / 👎.
miravoss26
left a comment
miravoss26
left a comment
There was a problem hiding this comment.
Adds an opt-in api.lan_auth house password for the LAN. Off by default; when on, protected routes (config, logs, dumps, driver source, writes) need Authorization: Bearer <house-password>. Loopback is always exempt (recovery path), and live status/energy/prices stay open as a viewer even without it.
What checks out:
- Password is argon2id in
state.db, notconfig.yaml. Confirmed via test that neither the hash nor the plaintext ever shows up in/api/configor the on-disk config file (TestGetConfigDoesNotContainLANPasswordHash). - Constant-time compare (
subtle.ConstantTimeCompare) on both the argon2 key and the Bearer token, no timing side-channel. - Fail-closed defaults verified:
apiMutationPolicy()leavesLANAuthEnabled/VerifyLANSecretnil by test, and the boot-phase listener starts with unbound closures reporting "off" untilstate.dbis open, so it can't lock you out mid-boot. - 5 wrong guesses locks all further attempts, including the correct password, for 30s. It's a single process-global counter by design, not per-IP, so any device on the LAN can trigger a repeatable 30s lockout for the real owner. Nuisance-level, not a data exposure, and it's called out as deliberate in the PR body.
One thing worth a question rather than a verdict: the very first "enable" is gated by the current (pre-toggle) state, which is off, so whichever device gets to /api/auth/password first sets the initial password with no proof required. That's bounded by the existing model (anyone on the LAN already has full owner access while lan_auth is off, so it's not a new hole), but it does mean the first password probably wants setting from loopback/console if the LAN isn't fully trusted yet at that moment. Docs don't call that out currently, might be worth a line.
CI is green, tests are thorough (481 new lines). Safe to merge from my read.
2 participants
Summary
Same change as #912, rebased onto current
master.#912 still carries the pre-merge #909 and #911 commits. Those were squash-merged, so GitHub reports CONFLICTING and cannot update the branch. This PR has only the LAN-auth commit on top of
master.api.lan_authis off by default. When on:Authorization: Bearer <house-password>127.0.0.1/::1) never does — that is recoverystate.db, not inconfig.yamlFTW_API_TOKENis still only for a public hostname. Two jobs, two secrets.Browser login is #913, which still sits on the old #912 tip and needs a restack after this lands.
Replaces
#912. Mira already read that PR and called it safe to merge once the stack underneath landed.
Test plan
go test ./go/internal/api/ ./go/internal/config/ ./go/cmd/ftw/GET /api/configfrom a LAN IP without Bearer is 401