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.
info@inference.coop is on the Members list but has no members row, so the
reconcile classified the co-op's own inbox as a stale subscriber and would have
unsubscribed it.
The data model already distinguishes this: OPERATOR_EMAILS (default
info@inference.coop,bot@inference.coop) are infrastructure accounts — the SSO
owner and the bot — which the panel surfaces under "admins" and excludes from
member counts. They live outside the members table by design. The Loomio
reconciliation applies the same rule to the same accounts via LOOMIO_ALWAYS_KEEP
("must always remain in the group").
So operators become the third class here: neither members nor stale. Their
subscription is left exactly as it is (never unsubscribed, and the reconcile
never subscribes, so an operator who unsubscribes stays unsubscribed). The
response reports how many were left, and the log line carries counts only.
The end-to-end probe against the live listmonk caught this: PUT
/api/subscribers/{id} with {"lists": []} returns 400 {"message":"Invalid
email."}, leaving the subscriber on the list. It needs {"email": ..., "lists":
[]}. The failed request was only a log warning, so the reconcile's main job —
taking lapsed members off the Members list — has never worked; only the DELETE
branch did anything, which is the branch this series just removed.
Now both classes unsubscribe successfully and keep their record.
Two findings from the retrospective review of 394cccfb..e24c272e (#34).
listmonk_reconcile() deleted subscribers with no members row outright. That
destroyed the subscriber's consent/unsubscribe history, and a stray test account
is indistinguishable from a real person who subscribed outside the portal. Both
classes now leave the list the same way — PUT {"lists": []} — and the record is
kept as a suppression marker; nothing is purged automatically. The summary log
line reports counts by reason instead of subscriber addresses. Subscriber
pagination also gains a short-page fallback so a missing `total` cannot truncate
the reconcile at page 1.
reconcile_memberships() called fetch_members() unguarded, so the documented
fail-safe — skip reconciliation rather than deactivate everyone missing from a
partial list — was unreachable: since fetch_members() now raises, the sweep
returned a 500 instead of {"status": "skipped"}. It now catches the failure and
returns reason "member_fetch_failed". The two webhook call sites are left
unguarded on purpose: a payment webhook should fail loudly so OC retries it, and
it self-heals.
Refs #34
Testing the previous commit against the live Loomio user list showed it would
also have renamed a member whose handle was chosen by hand (a working name
would have become the sanitized OIDC uid) — the uid is domain-shaped, so the
sanitizer produced a different, wrong target. The function is scoped to
Loomio's own auto-generated handles: the email local part with
non-alphanumerics stripped plus a random suffix. Gate on that shape.
Verified against the live user list (70 users) before building: exactly one
handle changes — the member whose username fix 422'd on every sweep — no
computed target violates Loomio's rule, and the hand-chosen handle is left
alone.
The username fix rejected a dot-containing OAuth uid and then substituted the
email local part, which carries the same dots — so Loomio answered 422
("Usernames must contain lower case letters, numbers, underscores, and internal
hyphens only") on every sweep, for the one member whose Cloudron username has a
dot. Normalize instead: disallowed characters become underscores, and leading
and trailing hyphens/underscores are stripped. Also log the Loomio user id
rather than the member's email.
Root cause of test accounts receiving the newsletter: LISTMONK_API_USER was
set EMPTY in the portal env - env-present beats the code default, so every
listmonk_sync authenticated with an empty user and 403'd silently. Fixed both
the env (set to portal-sync) and the code (empty env falls back to default).
New: listmonk_reconcile() - the Members list must mirror ACTIVE members.
Stale subscribers are removed: members marked inactive are taken off the list
(record kept as suppression marker); addresses that are not members at all
(test/deleted accounts) are deleted outright. Wired as step 4 of the nightly
membership sweep + POST /admin/newsletter/reconcile for on-demand runs.
LiteLLM requires unique key aliases, so re-creating a key with an existing
(email, name) pair 400s ('alias already exists') — breaking key rotation
from the dashboard. The stored LiteLLM hash lets us retire the old key
first; the ON CONFLICT upsert then tracks the new key.
Surfaced by the LibreChat sandbox work: the portal chat-key rotation for
an SSO member failed with exactly this 400.
inference-bot
Ubuntu