fix(relay): require credentials for deployment admin access - #4606
Open
jmecom wants to merge 5 commits into
Open
fix(relay): require credentials for deployment admin access#4606jmecom wants to merge 5 commits into
jmecom wants to merge 5 commits into
Conversation
Co-authored-by: Jordan Mecom <jm@squareup.com> Signed-off-by: Jordan Mecom <jm@squareup.com>
jmecom
marked this pull request as ready for review
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>
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
BUZZ_ADMIN_PASSWORDBUZZ_ADMIN_HOSTis setBUZZ_ADMIN_USERNAMEadminThe colon ban matters: the digest covers
username:password, so without it("oper:ator", "secret")and("oper", "ator:secret")would authenticate as eachother. Passwords may contain colons.
The runtime keeps only a SHA-256 digest and compares it in constant time
(
subtle::ct_eq); a hand-writtenDebugimpl redacts it.just admingeneratesand prints a random dev password when one isn't set.
Rollout
squareup/bb-publicsetsBUZZ_ADMIN_HOST=admin.buzz.xyzin production with nopassword 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 = Nonemeansis_admin_host()is false, the admin router is never mounted, andauthorize()returns
not_found— nothing admin-scoped is reachable, and the admin SPA bundle isnever served because
web_dirlives on theAdminConfigthat 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 upgradesget
404 relay: no community is configured for this host. Three things do stayreachable there without credentials, as they are on every host: the NIP-11 document
(fail-open by design),
/assets/*fromBUZZ_WEB_DIR(the public web bundle, not theadmin one), and the
/invite/<code>SPA shell — which returns the same bytes for everycode and so is not an enumeration oracle.
Consequence:
admin.buzz.xyzgoes dark on deploy untilBUZZ_ADMIN_PASSWORDreaches the pod. To restore it, follow the
BUZZ_GIT_HOOK_HMAC_SECRETpattern insquareup/bb-public:/services/buzz/service-secrets/admin-passwordto AWS Secrets Manager (us-west-2)secretKey: BUZZ_ADMIN_PASSWORDinexternalsecret-buzz-secrets.yamlsecretKeyRefenv entry to the chart's deployment template —the chart wires keys individually, there is no
envFromWatch startup logs for
admin dashboard disabledto confirm state either way.Testing
cargo clippy -p buzz-relay --all-targets -- -D warnings— cleancargo fmt --check— cleancargo test -p buzz-relay --lib— 833 passed; the 10 failures are pre-existingand infra-dependent (no local Postgres/Redis), an identical set to
mainjust test)Originating Buzz thread:
buzz://message?channel=3928fe05-df61-4b5d-b9c7-d623b9b10ea1&id=3c6c02312f763fbe0d2bfc33a6c1a362f91d0354f3d18b039cf7a0558c1439d1