security: separate admin token, verify credit-pack orders, revoke access on lapse
Member disclosure, three findings (fixes 1-3 of 5): 1. WEBHOOK_TOKEN was both the Open Collective webhook URL token and the X-Admin-Token for every /admin/* route, so anything that learned the webhook URL (OC admins, OC logs, this app's access logs) gained portal admin. The URL token is forced by OC's generic webhooks, which cannot set headers — the reuse is not. Introduce ADMIN_TOKEN, header-only. ADMIN_TOKEN_LEGACY=1 (default) still accepts WEBHOOK_TOKEN and logs a warning, so the admin panel and the scheduler keep working; set ADMIN_TOKEN everywhere, then flip it to 0 and rotate. sweep.sh and the digest script now send ADMIN_TOKEN. 2. The credit-pack path fell back to the webhook payload's own amount when OC could not resolve the order, so a forged POST naming its own totalAmount minted credits for any slug. The payload is now a hint only: an unresolvable order is refused, logged loudly, and emailed to the operator for manual reconciliation. Transient OC failures hold instead of granting blind. 3. Lapsing did not revoke access: both deactivation sites (lapse webhook, nightly sweep) discarded the key token deactivate_member() returns. Member keys carry no max_budget of their own, so they inherit the team's — zero the team budget on deactivation, which blocks every key under it. Key deletion is deliberately NOT used: provisioning reuses the stored sk- token on reactivation, and get_or_create_team does not update an existing team's budget, so the two renewal paths now re-sync the budget explicitly. Failures are loud (log + alert email); deactivation still completes.
This commit is contained in:
1 parent
e55d8a3f1e
commit
5720a63958
3 files changed
+110
-17
No files matched your search
@@ -27,5 +27,5 @@
|
|||||||
"manifestVersion": 2,
|
"manifestVersion": 2,
|
||||||
"website": "https://inference.coop",
|
"website": "https://inference.coop",
|
||||||
"contactEmail": "info@inference.coop",
|
"contactEmail": "info@inference.coop",
|
||||||
"dockerImage": "registry.inference.coop/co-op/member-portal:20261026-9495daa"
|
"dockerImage": "registry.inference.coop/co-op/member-portal:20261026-e55d8a3"
|
||||||
}
|
}
|
||||||
+101
-11
@@ -98,6 +98,17 @@ BROKER_SECRET = os.environ.get("BROKER_SECRET", "")
|
|||||||
# is the standard way to authenticate them.
|
# is the standard way to authenticate them.
|
||||||
WEBHOOK_TOKEN = os.environ.get("WEBHOOK_TOKEN", "")
|
WEBHOOK_TOKEN = os.environ.get("WEBHOOK_TOKEN", "")
|
||||||
|
|
||||||
|
# SECURITY (2026-10-27 member disclosure): the /admin/* token is SEPARATE from
|
||||||
|
# WEBHOOK_TOKEN. WEBHOOK_TOKEN must sit in the Open Collective webhook URL (OC's
|
||||||
|
# generic webhooks cannot send headers), so anything that learns that URL — an OC
|
||||||
|
# admin, OC's logs, this app's own access logs, a Referer — used to gain portal
|
||||||
|
# admin. ADMIN_TOKEN is only ever sent as a header and never appears in a URL.
|
||||||
|
# Cutover: while ADMIN_TOKEN_LEGACY is not "0" we also accept WEBHOOK_TOKEN on
|
||||||
|
# /admin/* so the admin panel, the scheduler and the digest scripts keep working.
|
||||||
|
# Set ADMIN_TOKEN everywhere, then set ADMIN_TOKEN_LEGACY=0 and rotate both.
|
||||||
|
ADMIN_TOKEN = os.environ.get("ADMIN_TOKEN", "")
|
||||||
|
ADMIN_TOKEN_LEGACY = os.environ.get("ADMIN_TOKEN_LEGACY", "1") != "0"
|
||||||
|
|
||||||
OC_GRAPHQL_URL = "https://opencollective.com/api/graphql/v2"
|
OC_GRAPHQL_URL = "https://opencollective.com/api/graphql/v2"
|
||||||
OC_COLLECTIVE_SLUG = os.environ.get("OC_COLLECTIVE_SLUG", "inference-cooperative")
|
OC_COLLECTIVE_SLUG = os.environ.get("OC_COLLECTIVE_SLUG", "inference-cooperative")
|
||||||
|
|
||||||
@@ -534,16 +545,21 @@ def _verify_portal_secret(request: Request) -> None:
|
|||||||
|
|
||||||
|
|
||||||
def _verify_admin_token(request: Request) -> None:
|
def _verify_admin_token(request: Request) -> None:
|
||||||
"""Authenticate admin/internal endpoints via the X-Admin-Token header.
|
"""Authorize a /admin/* caller (SECURITY 2026-10-27).
|
||||||
|
|
||||||
Replaces the old token-in-URL pattern (tokens in URLs leak into access
|
Admin auth is a HEADER, never a URL token: the webhook URL token lives in
|
||||||
logs and Referer headers). Callers pass the admin token in a header instead.
|
places we do not control, and it used to unlock all of /admin/*.
|
||||||
"""
|
"""
|
||||||
if not WEBHOOK_TOKEN:
|
|
||||||
raise HTTPException(503, "Admin token not configured")
|
|
||||||
provided = request.headers.get("x-admin-token", "")
|
provided = request.headers.get("x-admin-token", "")
|
||||||
if not secrets.compare_digest(provided, WEBHOOK_TOKEN):
|
if ADMIN_TOKEN and secrets.compare_digest(provided, ADMIN_TOKEN):
|
||||||
raise HTTPException(401, "Invalid admin token")
|
return
|
||||||
|
if ADMIN_TOKEN_LEGACY and WEBHOOK_TOKEN and secrets.compare_digest(provided, WEBHOOK_TOKEN):
|
||||||
|
logger.warning(
|
||||||
|
"admin auth: caller used the legacy WEBHOOK_TOKEN — move it to"
|
||||||
|
" ADMIN_TOKEN, then set ADMIN_TOKEN_LEGACY=0"
|
||||||
|
)
|
||||||
|
return
|
||||||
|
raise HTTPException(403, "Forbidden")
|
||||||
|
|
||||||
|
|
||||||
def get_db() -> sqlite3.Connection:
|
def get_db() -> sqlite3.Connection:
|
||||||
@@ -1851,6 +1867,51 @@ async def litellm_disable_key(key_token: str) -> None:
|
|||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
async def litellm_team_id_for_email(email: str) -> str | None:
|
||||||
|
"""The team_id of a member's budget team, or None if they have none."""
|
||||||
|
async with httpx.AsyncClient(timeout=httpx.Timeout(connect=15.0, read=60.0, write=15.0, pool=15.0)) as client:
|
||||||
|
r = await client.get(f"{LITELLM_BASE}/team/list",
|
||||||
|
headers={"Authorization": f"Bearer {LITELLM_MASTER_KEY}"})
|
||||||
|
r.raise_for_status()
|
||||||
|
for t in r.json():
|
||||||
|
if t.get("team_alias") == f"member:{email}":
|
||||||
|
return t["team_id"]
|
||||||
|
return None
|
||||||
|
|
||||||
|
|
||||||
|
async def revoke_member_litellm_access(email: str) -> bool:
|
||||||
|
"""Cut a deactivated member's gateway access (SECURITY 2026-10-27).
|
||||||
|
|
||||||
|
Zeroing the team budget blocks every key under it: member-managed keys carry
|
||||||
|
no max_budget of their own (verified live 2026-10-27), so they inherit the
|
||||||
|
team's. Deleting the key instead would break reactivation, which deliberately
|
||||||
|
reuses the stored sk- token — and get_or_create_team does NOT update the
|
||||||
|
budget of an existing team, so reactivation paths must re-sync explicitly.
|
||||||
|
|
||||||
|
Best-effort by design: a failure here must not stop deactivation from
|
||||||
|
completing, but it must be loud, because the member keeps access until it
|
||||||
|
succeeds.
|
||||||
|
"""
|
||||||
|
try:
|
||||||
|
team_id = await litellm_team_id_for_email(email)
|
||||||
|
if not team_id:
|
||||||
|
logger.info("Revoke: %s has no gateway team; nothing to zero", email)
|
||||||
|
return True
|
||||||
|
await litellm_update_team_budget(team_id, 0.0)
|
||||||
|
logger.info("Revoke: %s gateway budget zeroed (team %s)", email, team_id)
|
||||||
|
return True
|
||||||
|
except Exception as e:
|
||||||
|
logger.error("Revoke: FAILED to zero gateway budget for %s: %s", email, e)
|
||||||
|
try:
|
||||||
|
send_email("info@inference.coop",
|
||||||
|
"Deactivation did not revoke gateway access",
|
||||||
|
f"{email} was deactivated but their LiteLLM team budget could not be zeroed:\n\n{e}\n\n"
|
||||||
|
"Their existing API keys may still work. Fix by hand: set the team's max_budget to 0.\n")
|
||||||
|
except Exception:
|
||||||
|
pass
|
||||||
|
return False
|
||||||
|
|
||||||
|
|
||||||
# --- Broker: member-managed API keys (scoped to the member's team) ---
|
# --- Broker: member-managed API keys (scoped to the member's team) ---
|
||||||
|
|
||||||
def _api_key_alias(email: str, name: str) -> str:
|
def _api_key_alias(email: str, name: str) -> str:
|
||||||
@@ -2034,10 +2095,33 @@ async def opencollective_webhook(request: Request, token: str):
|
|||||||
# public order API. Fallback: webhook totalAmount (slightly
|
# public order API. Fallback: webhook totalAmount (slightly
|
||||||
# overgrants if a tip was included; logged).
|
# overgrants if a tip was included; logged).
|
||||||
tip_free_cents = await get_order_amount(order.get("idV2"))
|
tip_free_cents = await get_order_amount(order.get("idV2"))
|
||||||
if tip_free_cents is not None:
|
if tip_free_cents is None:
|
||||||
amount = round(tip_free_cents / 100.0, 2)
|
# SECURITY (2026-10-27): do NOT fall back to the payload's own
|
||||||
else:
|
# amount. This route is authenticated only by a URL token, so a
|
||||||
amount = round(float(amount_cents) / 100.0, 2)
|
# forged POST naming its own totalAmount used to mint credits for
|
||||||
|
# any slug. Treat the payload as a hint; require a resolvable OC
|
||||||
|
# order. A forged id cannot resolve; a transient OC failure holds
|
||||||
|
# for manual reconciliation instead of granting blind.
|
||||||
|
claimed = round(float(amount_cents) / 100.0, 2)
|
||||||
|
logger.warning(
|
||||||
|
"Credit pack REJECTED: OC could not resolve order %r (slug=%s, payload claimed $%.2f)"
|
||||||
|
" — no credits granted",
|
||||||
|
order.get("idV2"), slug, claimed,
|
||||||
|
)
|
||||||
|
try:
|
||||||
|
send_email(
|
||||||
|
"info@inference.coop",
|
||||||
|
"Credit pack could not be verified — no credits granted",
|
||||||
|
f"A credit-pack webhook arrived for {slug} claiming ${claimed:.2f}, but\n"
|
||||||
|
f"Open Collective's order API could not resolve order id {order.get('idV2')!r}.\n\n"
|
||||||
|
"No credits were granted: the payload's amount is not trusted. If this was a\n"
|
||||||
|
"real purchase, confirm the order in Open Collective and grant the credits\n"
|
||||||
|
"by hand; if it was not, this was an attempt to mint credits.\n",
|
||||||
|
)
|
||||||
|
except Exception as e:
|
||||||
|
logger.warning("Credit pack rejection: operator email failed: %s", e)
|
||||||
|
return JSONResponse({"status": "unverified_order_held", "slug": slug})
|
||||||
|
amount = round(tip_free_cents / 100.0, 2)
|
||||||
charged = round(float(amount_cents) / 100.0, 2)
|
charged = round(float(amount_cents) / 100.0, 2)
|
||||||
if charged > amount:
|
if charged > amount:
|
||||||
logger.info(
|
logger.info(
|
||||||
@@ -2131,6 +2215,9 @@ async def opencollective_webhook(request: Request, token: str):
|
|||||||
if first_payment:
|
if first_payment:
|
||||||
try:
|
try:
|
||||||
await provision_member(member["email"], member["name"], member["slug"])
|
await provision_member(member["email"], member["name"], member["slug"])
|
||||||
|
# A lapsed member's team budget was zeroed on deactivation and
|
||||||
|
# get_or_create_team does not restore it — re-apply explicitly.
|
||||||
|
await sync_team_budget_from_balance(member["email"])
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
logger.warning("Webhook: failed to provision %s: %s", member["email"], e)
|
logger.warning("Webhook: failed to provision %s: %s", member["email"], e)
|
||||||
try:
|
try:
|
||||||
@@ -2161,6 +2248,7 @@ async def opencollective_webhook(request: Request, token: str):
|
|||||||
if first_payment:
|
if first_payment:
|
||||||
try:
|
try:
|
||||||
await provision_member(org_email, name, slug)
|
await provision_member(org_email, name, slug)
|
||||||
|
await sync_team_budget_from_balance(org_email)
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
logger.warning("Webhook: org re-provision failed for %s: %s",
|
logger.warning("Webhook: org re-provision failed for %s: %s",
|
||||||
org_email, e)
|
org_email, e)
|
||||||
@@ -2200,6 +2288,7 @@ async def opencollective_webhook(request: Request, token: str):
|
|||||||
await cloudron_set_group(member["cloudron_user_id"], INACTIVE_GROUP_ID)
|
await cloudron_set_group(member["cloudron_user_id"], INACTIVE_GROUP_ID)
|
||||||
# Mark inactive in our DB (key injector will refuse requests)
|
# Mark inactive in our DB (key injector will refuse requests)
|
||||||
deactivate_member(member["email"])
|
deactivate_member(member["email"])
|
||||||
|
await revoke_member_litellm_access(member["email"])
|
||||||
await listmonk_sync(member["email"], subscribe=False)
|
await listmonk_sync(member["email"], subscribe=False)
|
||||||
await sync_loomio_memberships()
|
await sync_loomio_memberships()
|
||||||
await sync_matrix_space()
|
await sync_matrix_space()
|
||||||
@@ -3366,6 +3455,7 @@ async def reconcile_memberships() -> dict:
|
|||||||
if INACTIVE_GROUP_ID and user_id:
|
if INACTIVE_GROUP_ID and user_id:
|
||||||
await cloudron_set_group(user_id, INACTIVE_GROUP_ID)
|
await cloudron_set_group(user_id, INACTIVE_GROUP_ID)
|
||||||
deactivate_member(email)
|
deactivate_member(email)
|
||||||
|
await revoke_member_litellm_access(email)
|
||||||
await listmonk_sync(email, subscribe=False)
|
await listmonk_sync(email, subscribe=False)
|
||||||
deactivated_count += 1
|
deactivated_count += 1
|
||||||
logger.info("Sweep: deactivated %s", email)
|
logger.info("Sweep: deactivated %s", email)
|
||||||
|
|||||||
@@ -5,19 +5,22 @@
|
|||||||
# the portal against the live Open Collective member list: provisions missing
|
# the portal against the live Open Collective member list: provisions missing
|
||||||
# members, deactivates lapsed members, and syncs Loomio.
|
# members, deactivates lapsed members, and syncs Loomio.
|
||||||
#
|
#
|
||||||
# The WEBHOOK_TOKEN env var is shared with the main app (scheduler runs in the
|
# The ADMIN_TOKEN env var is shared with the main app (scheduler runs in the
|
||||||
# same launch environment), so we use it to authenticate the call.
|
# same launch environment), so we use it to authenticate the call. Admin auth is
|
||||||
|
# a header, never a URL token (security fix 2026-10-27). WEBHOOK_TOKEN is only a
|
||||||
|
# fallback until this caller is fully migrated.
|
||||||
#
|
#
|
||||||
# Uses Python (guaranteed present in the base image) rather than curl.
|
# Uses Python (guaranteed present in the base image) rather than curl.
|
||||||
|
|
||||||
set -e
|
set -e
|
||||||
|
|
||||||
if [ -z "$WEBHOOK_TOKEN" ]; then
|
ADMIN_TOKEN="${ADMIN_TOKEN:-$WEBHOOK_TOKEN}"
|
||||||
echo "sweep: WEBHOOK_TOKEN not set; aborting" >&2
|
if [ -z "$ADMIN_TOKEN" ]; then
|
||||||
|
echo "sweep: neither ADMIN_TOKEN nor WEBHOOK_TOKEN is set; aborting" >&2
|
||||||
exit 1
|
exit 1
|
||||||
fi
|
fi
|
||||||
|
|
||||||
python3 - "$WEBHOOK_TOKEN" <<'PY'
|
python3 - "$ADMIN_TOKEN" <<'PY'
|
||||||
import sys, urllib.request, urllib.error
|
import sys, urllib.request, urllib.error
|
||||||
|
|
||||||
token = sys.argv[1]
|
token = sys.argv[1]
|
||||||
|
|||||||
Reference in new issue
Block a user