feat(service): password-protect a service with --basic-auth

[mhenrixon]

Summary

kamal-proxy deploy service1 --target web-1:3000 --tls --host app.example.com --basic-auth admin:s3cr3t

Unauthenticated requests get 401 + WWW-Authenticate; correct credentials pass through. Closes #8.

Two decisions differ from what the issue specifies, both deliberate.

1. It is not a middleware

The issue says "middleware in createMiddleware (service.go:458)". Verified why that cannot be right:

createMiddleware wraps around serviceRequestWithTarget (service.go:382 dispatches into s.middleware), and the HTTPS redirect lives inside that innermost handler — handleRedirectsIfNeeded at service.go:682. So every middleware slot runs before the 301-to-HTTPS. A challenge issued from up there makes the browser send Authorization: Basic <base64> in the clear before the redirect ever fires.

The check therefore sits inline in serviceRequestWithTarget:

• after handleRedirectsIfNeeded — so credentials are never solicited over plaintext on a redirecting service;
• before handlePausedAndStoppedRequests — so protection does not lapse while a service is paused.

Upstream basecamp#216 challenges before the redirect. It is also mergeable: CONFLICTING, from a first-time contributor, with no maintainer comment — so this is a rewrite against current dash, not a cherry-pick.

2. The CLI hashes the credential

EncodeBasicAuthCredential emits sha256:<hex salt>:<hex digest>, so no plaintext crosses the RPC socket or reaches the state file. Salted per encode, so one leaked state file cannot confirm two services share a password.

Deliberately not bcrypt. Verification runs on the unauthenticated request path; a ~60ms adaptive hash there is an uncacheable CPU-exhaustion primitive handed to exactly the people the password exists to exclude. Comparison is subtle.ConstantTimeCompare over two fixed-length digests, so neither username nor password leaks its length. The sha256: prefix keeps an upgrade path open. Honest limitation: this is not an adaptive hash, so a weak password in a readable state file is still open to offline dictionary attack.

Also in this PR

Change	Why
Authorization stripped above the exemptions	Browsers replay cached credentials to every path in the protection space, including the health check one; otherwise that path forwards them upstream and into --log-request-header authorization
Health check path of / rejected when auth is on	That path is served without credentials, so it would publish the index. A stripped --path-prefix also resolves to /, so this covers both
Unreadable stored credential denies that one service	decodeStateServices does one json.Unmarshal over the whole slice and UnmarshalJSON calls initialize — returning an error there boots the proxy with zero services, and the .bak fails identically
State file 0644 → 0600, plus an explicit Chmod in writeFileAtomic	It now carries a credential digest, and every other secret-bearing artifact in the repo was already 0600

Gem contract (zoolutions/kamal#12)

The gem emits one flag: --basic-auth <username>:<password>. Split is at the first colon — passwords may contain colons, usernames may not, matching net/http's own decoder. Do not add it to rollout options: SetRolloutTargets reuses the stored service.options, so rollout traffic already inherits the credential.

Release ordering matters here more than usual. gob drops unknown fields silently, so a new CLI against an old proxy deploys the service completely unauthenticated with no error. Publish the proxy image first, then bump MINIMUM_VERSION.

Test plan

• make test, go vet ./..., gofmt -l internal/ cmd/ clean
• go test -race ./internal/server/ ./internal/cmd/ — 660 tests, clean
• make build + kamal-proxy deploy --help shows the flag
• The 401/200/401 triple the issue asks for: TestBasicAuth_ChallengesUnauthenticatedRequests, _AllowsCorrectCredentials, _RejectsWrongCredentials
• TestBasicAuth_RedirectsBeforeChallenging — the core security property
• TestBasicAuth_ChallengeSurvivesInterceptErrors — --intercept-errors=401 does not strip the challenge header
• TestBasicAuth_StateWrittenBeforeTheOptionStaysUnprotected — old state files load open, never locked out
• TestRouter_StateFileIsNotWorldReadable

Four mutations were applied to the implementation and confirmed to fail their tests, so these prove behavior rather than plumbing:

Mutation	Caught by
Auth check moved above the redirect (the basecamp#216 mistake)	TestBasicAuth_RedirectsBeforeChallenging — "Should be empty, but was Basic realm=…"
r.Header.Del("Authorization") removed	_StripsAuthorizationBeforeForwarding, _OnExemptPaths
matches() always true	TestBasicAuthCredential_EncodeAndMatch (4 cases), _RejectsWrongCredentials (4 cases)
Unreadable stored credential fails open	_UnreadableStoredCredentialFailsClosed

Known gaps, documented not fixed

1. The password is in the deploy host's process table. Ordinary argv, visible to ps. Closing it needs both repos to adopt stdin — cross-repo, not size:S.
2. Rolling the proxy image back silently removes protection. An older binary ignores the stored credential and the next state save drops it permanently. README says to redeploy after a rollback.
3. --tls-redirect=false and non-TLS services are challenged over plaintext. Warned at deploy, not rejected — the fork's own gem deletes --tls from deploy_options when load-balancing, so a hard error would break every load-balanced deployment that enables auth.
4. Prefix routing is not a security boundary. serviceFor matches raw r.URL.Path with no normalization, so //admin and /./admin fall through to a sibling service. Documented in the README.
5. Unlocked read of s.basicAuth in serviceRequestWithTarget, same as s.options/s.middleware today. Pre-existing pattern; credential rotation under traffic is not atomic. Worth its own issue.

Deviations & judgment calls

Design came from a multi-agent pass and three adversarial reviews; the reviews found seven real problems, all folded in before any code was written.

• --tls --tls-redirect=false challenges in cleartext, silently. redirectURLIfNeeded only upgrades when TLSEnabled && TLSRedirect, so the "structurally closed" claim held only for redirecting services. Warning widened to !TLSEnabled || !TLSRedirect. I rejected the reviewer's second half — suppressing WWW-Authenticate on that path — because ssl_redirect: false is exactly the TLS-terminated-upstream topology, where suppressing would break basic auth outright for those users. Warn and document beats silently half-working.
• Authorization strip was below the exemption early-return. Hoisted above both.
• A health check path of / would have exposed the protected index. Nothing validates that path; now rejected at deploy when auth is on.
• Unparseable stored credential took down the entire state file. Now fails closed for that one service.
• The design's request tests asserted a body the harness cannot produce. testRouter/sendGETRequest drive router.ServeHTTP with no ErrorPageMiddleware, so SetErrorResponse takes the http.Error fallback. Added a shared testRoutedHandler helper that wraps the router the way Server.buildHandler does.
• A test caught its own bug first. The "target never reached" counter failed with 1, because DeployService health-checks the backend during deploy. Noted because the same effect would flatter a naive version of that assertion into passing for the wrong reason.
• Dropped the design's exported BasicAuthCredentialMatches test helper rather than add production surface for tests. The CLI's real responsibility — cutting at the first colon — is extracted as parseBasicAuthFlag and table-tested; encoding is covered server-side.
• Dropped TestBasicAuth_DoesNotBlockACMEChallenges — not constructible as specified (no seam to inject a CertManager; forcing TLSEnabled makes the control request a 301, not the asserted 401). TestBasicAuth_ExemptsInternalRequests covers the same isInternalRequest branch the ACME and TLS on-demand probes travel through.
• One opaque field, not upstream's BasicAuthUsername + BasicAuthPasswordHash. Makes the half-configured state (username set, hash empty ⇒ permanent 401) unrepresentable, keeps the username off disk, and means one digest and therefore one compare — no username timing oracle to write.
• Per-path scoping not shipped. --path-prefix already deploys a distinct Service with its own ServiceOptions and therefore its own credential; a rule list would have fail-open polarity.
• No internal/pages/401.html. It would be the first fork-only asset in an upstream-owned directory, and it does not help the --error-pages-without-401.html case anyway, since the service-level middleware only parses the operator's directory. README says to add one.
• State file 0644 → 0600 touches router.go and util.go, both upstream-owned, so it widens merge surface — three tokens plus a Chmod, accepted because this change is what puts a credential digest in that file.