Skip to content

fix(relay): require credentials for deployment admin access - #4606

Open
jmecom wants to merge 5 commits into
mainfrom
codex/security-admin-host-origin-auth
Open

fix(relay): require credentials for deployment admin access#4606
jmecom wants to merge 5 commits into
mainfrom
codex/security-admin-host-origin-auth

Conversation

@jmecom

@jmecom jmecom commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator

This change requires HTTP Basic credentials for every deployment admin route: the
dashboard, the admin SPA bundle it loads from BUZZ_ADMIN_WEB_DIR, the admin APIs,
reports, feedback, and attachments.
Unauthenticated requests get 401 with WWW-Authenticate: Basic realm="buzz-admin",
so browsers hitting the dashboard now see a credential prompt.

The existing Host and Origin checks remain as boundary checks, but they no longer
act as the administrator credential.

Configuration

Variable Required Rule
BUZZ_ADMIN_PASSWORD yes, when BUZZ_ADMIN_HOST is set 16–1024 bytes, no control characters
BUZZ_ADMIN_USERNAME no, defaults to admin ≤128 chars, no colon, no control characters

The colon ban matters: the digest covers username:password, so without it
("oper:ator", "secret") and ("oper", "ator:secret") would authenticate as each
other. Passwords may contain colons.

The runtime keeps only a SHA-256 digest and compares it in constant time
(subtle::ct_eq); a hand-written Debug impl redacts it. just admin generates
and prints a random dev password when one isn't set.

Rollout

squareup/bb-public sets BUZZ_ADMIN_HOST=admin.buzz.xyz in production with no
password configured. An earlier revision of this PR aborted startup on that
config, which would have crashlooped the entire relay fleet — websocket, media,
git — over an optional dashboard.

The relay now logs an error and disables the dashboard instead, so the image
is safe to ship in any order relative to the config change. admin = None means
is_admin_host() is false, the admin router is never mounted, and authorize()
returns not_found — nothing admin-scoped is reachable, and the admin SPA bundle is
never served because web_dir lives on the AdminConfig that is now absent.

In that state the admin host is treated like any unmapped host. It is not a community
subdomain, so bind_community() fails and both browser requests and WebSocket upgrades
get 404 relay: no community is configured for this host. Three things do stay
reachable there without credentials, as they are on every host: the NIP-11 document
(fail-open by design), /assets/* from BUZZ_WEB_DIR (the public web bundle, not the
admin one), and the /invite/<code> SPA shell — which returns the same bytes for every
code and so is not an enumeration oracle.

Consequence: admin.buzz.xyz goes dark on deploy until BUZZ_ADMIN_PASSWORD
reaches the pod. To restore it, follow the BUZZ_GIT_HOOK_HMAC_SECRET pattern in
squareup/bb-public:

  1. Add /services/buzz/service-secrets/admin-password to AWS Secrets Manager (us-west-2)
  2. Map it to secretKey: BUZZ_ADMIN_PASSWORD in externalsecret-buzz-secrets.yaml
  3. Add the matching secretKeyRef env entry to the chart's deployment template —
    the chart wires keys individually, there is no envFrom

Watch startup logs for admin dashboard disabled to confirm state either way.

Testing

  • cargo clippy -p buzz-relay --all-targets -- -D warnings — clean
  • cargo fmt --check — clean
  • cargo test -p buzz-relay --lib — 833 passed; the 10 failures are pre-existing
    and infra-dependent (no local Postgres/Redis), an identical set to main
  • Not run locally: integration suite (just test)

Originating Buzz thread: buzz://message?channel=3928fe05-df61-4b5d-b9c7-d623b9b10ea1&id=3c6c02312f763fbe0d2bfc33a6c1a362f91d0354f3d18b039cf7a0558c1439d1

Co-authored-by: Jordan Mecom <jm@squareup.com>
Signed-off-by: Jordan Mecom <jm@squareup.com>
@jmecom
jmecom marked this pull request as ready for review August 3, 2026 21:00
@jmecom
jmecom requested a review from a team as a code owner August 3, 2026 21:00
`authorize_request` returned `Result<(), Response>`, tripping
`clippy::result_large_err` (the axum `Response` Err variant is >=128
bytes). Return `ApiError` instead and convert at the two call sites.

Also drops a redundant closure in the config test.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: Eli Foster <efoster@squareup.com>
elifoster-block and others added 3 commits August 3, 2026 15:53
Follow-up to dc2df16: moving the response conversion out to the call
sites left `IntoResponse` unused in the admin module.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: Eli Foster <efoster@squareup.com>
`admin_host_requires_a_strong_credential` set BUZZ_ADMIN_HOST with no
password to assert the rejection. Env vars are process-global, and the
~15 other tests that call `Config::from_env()` do not hold ENV_MUTEX, so
they observed the half-set state and panicked on `expect("default config
loads")`.

Extract the validation into `AdminConfig::from_values` and test the
rejection cases against it directly, with no env mutation. A separate
test still covers the env wiring; it sets the password before the host so
a concurrent `from_env()` never sees a host without one.

Repro before this change:
  cargo test -p buzz-relay --lib -- \
    api::admin::tests::report_detail_requires_admin_host_before_database_access \
    config::tests::admin_host_requires_a_strong_credential
  => 1 passed; 1 failed

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: Eli Foster <efoster@squareup.com>
…startup

BUZZ_ADMIN_HOST with a missing or invalid password aborted relay startup.
bb-public sets BUZZ_ADMIN_HOST=admin.buzz.xyz in production with no
password configured, so shipping this would have crashlooped the whole
relay fleet — websocket, media, git, everything — over an optional
dashboard.

Log at error! and leave the admin surface off instead. `admin = None`
means is_admin_host() is false, the admin router is never mounted, and
authorize() returns not_found, so nothing admin-scoped is reachable. The
security posture is unchanged; only the blast radius of a config gap is.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: Eli Foster <efoster@squareup.com>
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