Skip to content

fix(serve): refuse foreign Host/Origin and require token on control routes - #6

Open
rizperdana wants to merge 6 commits into
mainfrom
release/phase1-loopback-hardening
Open

rizperdana wants to merge 6 commits into
mainfrom
release/phase1-loopback-hardening

Conversation

@rizperdana

Copy link
Copy Markdown
Owner

What changed

Host/Origin gating (internal/serve/serve.go) — the HTTP server now refuses requests whose Host or Origin header does not match the address it is actually bound to. Cross-origin and DNS-rebinding attempts are rejected with 421 Misdirected Request instead of being served.

Control routes always require the bearer token — the operational/control endpoints (/-/load, /-/stop, and friends) no longer treat a loopback origin as implicit authorization. The bearer token is required unconditionally, answering 401 when absent or wrong.

load.go token fix — a d.token dereference was reading the wrong receiver, so the token was not actually being propagated to the request. Corrected.

What was verified

  • The 4 gate tests added to internal/serve/serve_test.go cover: foreign Host → 421, foreign Origin → 421, control route without token → 401, control route with token → 200.
  • A live end-to-end transcript on 127.0.0.1:5199 exercising real responses: 421, 403, 401, 400, 200, and 502 — all observed, not assumed.
  • go build ./... and go vet ./... are clean.

Knowingly incomplete

Two documentation defects were open at the time this branch was cut:

  1. AGENTS.md repo map was missing internal/selfmgmt/.
  2. docs/screenshots/run.txt:53-59 showed an unattributed /-/load call that now correctly returns 401.

Both are addressed by commit f24da46 on this branch, which is included here.

Note on CI

Two tests in internal/cli fail on this branch: TestServePositionalModelAttachNoteAndUsage and TestServeModelNoRuntimeOneRemedyBlock. These failures are pre-existing — they fail identically on origin/main (a1520ee) and are not caused by this work. internal/cli is owned by a separate in-flight change that is not part of this branch.

…outes

Two header gates now run before any handler: Host must name loopback (421 invalid_request, loopback only) and Origin, when present, must name loopback or the configured bind host (403 browser_origin_forbidden, cross-origin request refused). /healthz stays bearer-exempt but is no longer gate-exempt.

/-/status, /-/load and /-/unload always require the daemon state token, loopback included; only the chat surface stays bearer-free on a loopback bind. initialLoad attaches the token whenever one exists (was: only when needToken), so our own /-/load no longer 401s against a loopback daemon.

Tests: TestRejectsForeignHost, TestRejectsForeignOrigin, TestControlRoutesRequireTokenOnLoopback; TestInitialLoadThroughOwnLoadEndpoint is the load.go:147 regression proof (fails on the old condition, passes on the fix).

Docs: ARCHITECTURE auth contract rewritten; the two phantom contract-test claims (pin-bump gate, release gate) marked planned because no integration-tagged file exists; AGENTS gains the loopback-not-open invariant and drops the stale CONTRIBUTING.md claim.
…osis

Audit follow-up to e287e04.

AGENTS.md repo map gained internal/selfmgmt/ (self-update, uninstall, version).

docs/screenshots/run.txt showed a bare curl against /-/load answering 409; that
request now gets 401 because /-/load always requires the state token. The raw
capture cannot be re-taken (no live supervised daemon; port 5002 is off-limits),
so it is replaced by a prose note stating the 409 attach_mode contract, the
token requirement, and that the old capture is no longer reproducible — no new
transcript is claimed. docs/QUICKSTART.md now says the same, and both drop the
stale exit=1 claim for a mismatched attach-mode model name.

classifyListener/statusAnswers: a 401 from /-/status is our own control route
refusing an unauthenticated probe (salvaged daemon.json without a token), so a
new probeRefused class reports our daemon behind damaged state instead of
' a foreign process holds <addr>'. Fail-closed identification is unchanged —
any other non-200 is still probeForeign. All four diagnosis sites gained the
case; TestProbeRefusedNamesOurDaemonNotForeign covers 401 and keeps 404
classified as foreign.

Query names a rejected bearer instead of a bare 'status endpoint: HTTP 401'.
… engine

- cli: --continue/--history with mtime-ordered sessions and destructive
  context-overflow trim (internal/cli/history.go)
- serve: /api/{chat,tags,show,generate} ollama shim; honest omissions
  (no digest, unobservable durations are 0)
- serve: Backend seam (tabby|llama) wired at Serve; /-/load refuses on
  llama (no hot-swap) instead of falling back
- serve: draft settings ride boot config.yml draft_model; engine_env
  reaches the child; default drafting is OFF and unmeasured
- serve: TestAttachStreamsZeroBuffer made causal (no stopwatch)
- runner: pure-Go chat-template engine, byte-equal to transformers
  5.13.1 across 43 golden cases
- docs: README/ARCHITECTURE/QUICKSTART/AGENTS + site fact fixes
…hful

- doctor: Ready() and HasNVIDIA() had identical bodies; every caller
  meant "has a usable NVIDIA GPU", so Ready() is deleted and callers
  use HasNVIDIA(). Report.Device was never populated — removed.
- doctor: Detect() gains hermetic tests incl. a CPU-only case.
- preflight: GGUF refusal no longer contradicts the llama backend that
  now exists; it discloses the backend as experimental and unproven.
- docs: ARCHITECTURE synced to the refusal wording.
On a brand-new state dir, GET /v1/models returned 500 with a child
traceback: TabbyAPI lists model_dir from its first request and the
directory did not exist. Serve now creates it after backend selection
(so a bad --backend still writes nothing) and before Prepare renders
the config pointing at it. Regression test is hermetic and fails
against the old behaviour.
.omp/ holds agent skill/state files (SKILL.md), not project source; it was showing as untracked in every git status.
Comment thread internal/runner/expr.go
if err != nil {
return "", err
}
b.WriteByte(byte(v))
Comment thread internal/runner/expr.go
if err != nil {
return "", err
}
b.WriteString(string(rune(v)))
Comment thread internal/runner/expr.go
if err != nil {
return "", err
}
b.WriteString(string(rune(v)))

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants