Skip to content

Verify email addresses, fail closed on hotdata 404s, drop the shared ingest credential - #47

Merged
eddietejeda merged 4 commits into
mainfrom
tenant-isolation-hardening
Oct 17, 2026
Merged

eddietejeda merged 4 commits into
mainfrom
tenant-isolation-hardening

Conversation

@eddietejeda

@eddietejeda eddietejeda commented Sep 17, 2026 •

Copy link
Copy Markdown
Contributor

Review of tenant isolation found no cross-org data path, but three ways an
outsider could get in or stay in:

  • Team links restricted to a domain were cosmetic: addresses were never
    verified, so anyone holding the link could type any @Domain address and
    read the org. Every new account now has to open a mailed link while
    signed in as that account before it can see or report anything. An
    unverified address can be registered over (new password, squatter's
    sessions revoked), which also closes pre-registration squatting.
    Accounts older than the cutoff are trusted; with no mail key configured
    the gate is off and the server says so at boot.
  • rows() read a hotdata 404 as an empty table, so a transient 404 during a
    join made the joiner admin of an established org, and one during
    delete-org removed an org with members. Only a 400 is empty now.
  • The shared HOTUSAGE_INGEST_TOKEN could report usage as anyone in any org.
    It is now only the switch that requires a token; the value is accepted
    as a credential only with HOTUSAGE_ALLOW_SHARED_INGEST=1.

Also: an org admin can no longer delete a platform operator's account by
removing them, and database ids no longer appear in any response.

Breaking: /ingest with the shared token returns 401 unless HOTUSAGE_ALLOW_SHARED_INGEST=1. Per-user tokens (every signed-in client) are unaffected.

Email verification turns on only when RESEND_API_KEY and HOTUSAGE_EMAIL_FROM are set; without them the server logs that verification is off and behaves as before.

Tests: all suites pass, with new coverage for the gate, takeover, rows() 400-vs-404, the shared token, and the sysadmin removal guard.

…g the shared ingest token as a credential

Review of tenant isolation found no cross-org data path, but three ways an
outsider could get in or stay in:

- Team links restricted to a domain were cosmetic: addresses were never
  verified, so anyone holding the link could type any @Domain address and
  read the org. Every new account now has to open a mailed link while
  signed in as that account before it can see or report anything. An
  unverified address can be registered over (new password, squatter's
  sessions revoked), which also closes pre-registration squatting.
  Accounts older than the cutoff are trusted; with no mail key configured
  the gate is off and the server says so at boot.
- rows() read a hotdata 404 as an empty table, so a transient 404 during a
  join made the joiner admin of an established org, and one during
  delete-org removed an org with members. Only a 400 is empty now.
- The shared HOTUSAGE_INGEST_TOKEN could report usage as anyone in any org.
  It is now only the switch that requires a token; the value is accepted
  as a credential only with HOTUSAGE_ALLOW_SHARED_INGEST=1.

Also: an org admin can no longer delete a platform operator's account by
removing them, and database ids no longer appear in any response.
@eddietejeda
eddietejeda force-pushed the tenant-isolation-hardening branch from 220e6cf to f51ea8d Compare September 17, 2026 19:55
Comment thread server/server.py Outdated
Comment on lines +777 to +784
if user and self.verification and not self.is_verified(email):
# an unverified row is a claim on the address, not an account (see
# create_account): the acceptor sets the password and the mailbox
# decides who really joined
if not password:
raise ValueError("choose a password")
self.take_over(email, password)
self.join_org(email, inv["org_slug"])

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Take the existing-account join path when as_user already equals email. A session as that account already proves the password, so no takeover is needed. Also compute the invite page's MODE from "exists AND verified", not from get_user alone (server.py:2842).

Failure scenario: bob@acme.com registers at /register, so the row exists and is unverified. An admin then sends bob a single-use invite. GET /invite/<token> reaches server.py:2842, finds a user row, and sets MODE to join (bob signed in) or login (bob not signed in). invite.html:64-79 hides the password field in join mode and hides the whole form in login mode. Bob posts with no password, this branch raises choose a password, and the redirect lands back on the same page with the field still hidden. Bob can never accept the invite. The takeover branch added here is unreachable through the UI for single-use invites.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in a74f229. accept_invite takes the join path first whenever as_user equals the invited address, verified or not; the takeover branch only runs for a stranger posting a password. The invite page now sets MODE to join for a matching session, login only for an existing verified account, and new otherwise, so an unverified claim gets the password field. Test: test_a_signed_in_unverified_account_joins_through_a_single_use_invite.

Comment thread server/server.py Outdated
Comment on lines +1555 to +1588
# A verified answer never reverts, so it is kept for the life of the
# process; an unverified one is re-read often, since the next request
# after the click should see the change.
VERIFIED_NEG_TTL = 15
# Accounts created before this predate verification and are trusted as
# they were. Everyone after it proves the mailbox before seeing or
# reporting anything. The instant the change shipped, not a round date:
# a later cutoff would grandfather whoever registered in the gap.
VERIFICATION_SINCE = datetime(2026, 9, 17, 19, 0, tzinfo=timezone.utc)

@staticmethod
def _as_datetime(value):
if isinstance(value, str):
try:
value = datetime.fromisoformat(value.replace("Z", "+00:00"))
except ValueError:
return None
if not isinstance(value, datetime):
return None
if value.tzinfo is None:
value = value.replace(tzinfo=timezone.utc)
return value

def is_verified(self, email):
"""Has this address been proved -- or does it predate the requirement?
Always True with verification off."""
if not self.verification:
return True
email = email.strip().lower()
now = time.time()
with self.lock:
hit = self.verified_cache.get(email)
if hit and (hit[0] or now - hit[1] < self.VERIFIED_NEG_TTL):
return hit[0]

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Give the positive cache entry a TTL as well. A verified answer never reverts for one address, but the row can be deleted and the address re-registered by someone else.

Suggested change
# A verified answer never reverts, so it is kept for the life of the
# process; an unverified one is re-read often, since the next request
# after the click should see the change.
VERIFIED_NEG_TTL = 15
# Accounts created before this predate verification and are trusted as
# they were. Everyone after it proves the mailbox before seeing or
# reporting anything. The instant the change shipped, not a round date:
# a later cutoff would grandfather whoever registered in the gap.
VERIFICATION_SINCE = datetime(2026, 9, 17, 19, 0, tzinfo=timezone.utc)
@staticmethod
def _as_datetime(value):
if isinstance(value, str):
try:
value = datetime.fromisoformat(value.replace("Z", "+00:00"))
except ValueError:
return None
if not isinstance(value, datetime):
return None
if value.tzinfo is None:
value = value.replace(tzinfo=timezone.utc)
return value
def is_verified(self, email):
"""Has this address been proved -- or does it predate the requirement?
Always True with verification off."""
if not self.verification:
return True
email = email.strip().lower()
now = time.time()
with self.lock:
hit = self.verified_cache.get(email)
if hit and (hit[0] or now - hit[1] < self.VERIFIED_NEG_TTL):
return hit[0]
# Both answers expire. A verified answer never reverts for one address,
# but remove_user deletes the row and the address may be registered again
# by someone else -- and a delete is seen only by the instance that served
# it, so a positive entry cached forever on another instance would let the
# next registrant inherit it. An unverified answer is re-read often, since
# the next request after the click should see the change.
VERIFIED_TTL = 60
VERIFIED_NEG_TTL = 15
# Accounts created before this predate verification and are trusted as
# they were. Everyone after it proves the mailbox before seeing or
# reporting anything. The instant the change shipped, not a round date:
# a later cutoff would grandfather whoever registered in the gap.
VERIFICATION_SINCE = datetime(2026, 9, 17, 19, 0, tzinfo=timezone.utc)
@staticmethod
def _as_datetime(value):
if isinstance(value, str):
try:
value = datetime.fromisoformat(value.replace("Z", "+00:00"))
except ValueError:
return None
if not isinstance(value, datetime):
return None
if value.tzinfo is None:
value = value.replace(tzinfo=timezone.utc)
return value
def is_verified(self, email):
"""Has this address been proved -- or does it predate the requirement?
Always True with verification off."""
if not self.verification:
return True
email = email.strip().lower()
now = time.time()
with self.lock:
hit = self.verified_cache.get(email)
ttl = self.VERIFIED_TTL if hit and hit[0] else self.VERIFIED_NEG_TTL
if hit and now - hit[1] < ttl:
return hit[0]

Failure scenario: bob@x.dev is verified, and instances A and B both cache True with no expiry. An org admin removes bob, whose only org that was, so remove_from_org calls remove_user (server.py:1012) and deletes the verified_emails row. remove_user pops verified_cache only on the instance that served the request. An attacker registers bob@x.dev, and every request routed to instance B reads the stale True and passes _gated_on_verify. The attacker reads and reports without ever holding the mailbox. TOKEN_CACHE_TTL (server.py:623-628) caps the session cache for exactly this reason: App Runner runs several instances and a row delete is seen by one of them.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in a74f229: VERIFIED_TTL = 60 for positive answers, matching TOKEN_CACHE_TTL, with the suggested comment. Test: test_a_verified_answer_expires_from_the_cache deletes the row behind the cache and checks the stale True ages out.

Comment thread server/server.py Outdated
the previous claimant held is revoked, so they cannot ride along once
the real owner verifies. Callers check is_verified first."""
email = email.strip().lower()
self.set_password(email, password)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Drop the address's org memberships and admin grants inside take_over, and clear the account's active org. A membership the previous claimant acquired must not pass to the real owner. remove_user already does exactly this at server.py:1047-1050, for the reason its own comment gives at server.py:1051: whoever re-registers an address must not inherit a grant. accept_invite and accept_team_invite call join_org right after take_over, so an invite-driven join still happens.

Failure scenario: mallory holds a verified account and an org evil, and creates a team link with no domain restriction. Mallory posts /invite/<token> with email=victim@corp.com and any password. accept_team_invite reaches the final else and calls create_user, so victim@corp.com becomes an unverified member of evil with org_slug=evil. Victim later registers at /register. create_account (server.py:690) calls take_over, which changes only the password and the sessions. Victim opens the mailed link, verifies, and lands on evil's dashboard. Every collector report from victim routes into evil's database, and mallory reads it as an admin of evil.

Before this change the same pre-registration only locked victim out ("that email is already registered"). The takeover turns that lockout into a silent capture, so the fix belongs in the same commit.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in a74f229. take_over now drops every membership and admin grant and writes the users row with org_slug empty, then revokes sessions and clears route_cache. accept_invite and accept_team_invite join their org right after, as before. Test: test_a_takeover_inherits_nothing_the_squatter_acquired runs the exact team-link squat and checks the victim lands in no org.

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review

Blocking Issues

  1. server/server.py:777-784 — unchanged from cycle 1 (thread 4040902695). A single-use invite for an unverified address cannot be accepted through the UI. The invite page computes MODE from get_user alone at server.py:2842, so the password field is hidden, and the takeover branch then demands a password.

  2. server/server.py:1587 — unchanged from cycle 1 (thread 4040904642). A positive is_verified answer is cached with no expiry. remove_user pops the cache only on the instance that served the delete, so another instance keeps serving True for an address that someone else has since registered.

  3. server/server.py:1608-1621 — new (thread on the take_over body). take_over keeps the previous claimant's org memberships and active org. A pre-registered address therefore hands the real owner a membership in the pre-registrant's org.

Action Required

  • Take the existing-account join path in accept_invite when as_user already equals email. Compute the invite page's MODE from exists AND verified.
  • Give the positive verified_cache entry a TTL, as TOKEN_CACHE_TTL does at server.py:623-628.
  • Delete the address's org_memberships rows and admin grants in take_over, and clear users.org_slug.
  • Add a test for each of the three paths.

No new commits landed since cycle 1, so findings 1 and 2 stand on the code as written.

A takeover now drops every org membership, admin grant and the active org
the previous claimant acquired, so pre-registering a victim's address
through a team link no longer routes the victim's reports into the
squatter's org. Invite-driven callers join their org right after, as
before.

A positive is_verified answer expires after a minute like the session
cache: remove_user's row delete is seen by one instance, and the next
registrant of that address must not inherit a verified answer cached
forever on another.

A single-use invite for an unverified address is acceptable again: signed
in as that address it is a plain join (the session already proved the
password), and the invite page only hides the password field for a real
account or a matching session, so an unverified claim gets the field the
takeover branch needs.
@eddietejeda
eddietejeda requested a review from a team as a code owner October 17, 2026 02:34
@eddietejeda
eddietejeda requested review from anoop-narang and removed request for a team October 17, 2026 02:34
Comment thread server/server.py
Comment on lines +2380 to +2387
def established(addr):
# an unverified row is a claim, not an account: the acceptor
# sets a password and takes it over (see AuthStore.take_over)
addr = (addr or "").strip().lower()
return bool(addr and self.auth.get_user(addr)
and self.auth.is_verified(addr))
joining_existing = established(inv["email"]) if inv \
else established(form.get("email", ""))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Treat the address as established when the signed-in viewer is that address. accept_invite now takes the plain-join path in that case, but the password check here still runs first.

Suggested change
def established(addr):
# an unverified row is a claim, not an account: the acceptor
# sets a password and takes it over (see AuthStore.take_over)
addr = (addr or "").strip().lower()
return bool(addr and self.auth.get_user(addr)
and self.auth.is_verified(addr))
joining_existing = established(inv["email"]) if inv \
else established(form.get("email", ""))
def established(addr):
# an unverified row is a claim, not an account: the acceptor
# sets a password and takes it over (see AuthStore.take_over).
# Signed in as the address, the session is the proof instead.
addr = (addr or "").strip().lower()
return bool(addr and self.auth.get_user(addr)
and (addr == (as_user or "").strip().lower()
or self.auth.is_verified(addr)))
joining_existing = established(inv["email"]) if inv \
else established(form.get("email", ""))

Failure scenario: bob@acme.com registers and stays unverified. An admin sends bob a single-use invite. Bob, signed in, opens /invite/<token>. The GET handler sets MODE to join, so invite.html hides the password field. Bob posts with an empty password. established("bob@acme.com") returns False because is_verified is False. Line 2388 raises "password must be at least 8 characters" before accept_invite runs. The redirect shows the same page with the field still hidden, so bob cannot accept the invite. If bob gets past that check, line 2407 calls login(email, ""), which also fails.

The new test test_a_signed_in_unverified_account_joins_through_a_single_use_invite calls AuthStore.accept_invite directly, so it does not cover this handler. With the fix, lines 2400–2402 skip the resend and redirect to /, which is correct for a plain join.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in fa0f55e with the suggested established(). HTTP-level test test_an_unverified_invitee_accepts_their_own_invite_without_a_password in test_install_gate.py: signed in as the unverified invitee, GET shows join mode, POST with no password is a 302 to / and bob is a member and still signed in; a stranger gets the password field and is refused without one.

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review — Blocking Issues

  • server/server.py:2380-2388 — _handle_invite_accept still requires a password of 8+ characters when a signed-in, unverified account accepts its own single-use invite. established() returns False for an unverified address, so the check at line 2388 fails before accept_invite runs. The invite page is in join mode and hides the password field, so the invite cannot be accepted through the UI. The prior finding (thread 4040902695) is fixed in AuthStore.accept_invite and the GET handler, but not in the POST handler.

Action Required

  • In established(), return True when the address exists and equals the signed-in viewer's email.
  • Add an HTTP-level test that posts /invite/<token> with no password while signed in as an unverified invitee.

The other two prior findings (the positive cache TTL and take_over dropping memberships) are fixed.

…password

The POST handler's established() check treated an unverified row as a
claim needing a password, even when the viewer was signed in as that
address, so the join-mode page (which hides the field) could never submit.
A matching session is the proof instead. An HTTP-level test drives the
page and the POST as a browser, for the invitee and for a stranger.
Comment thread server/server.py Outdated
raise ValueError(f"this link only accepts @{inv['domain']} addresses")
if self.get_user(email):
user = self.get_user(email)
if user and self.verification and not self.is_verified(email):

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In accept_team_invite, take the plain-join path first when as_user equals email. This mirrors accept_invite. Since fa0f55e, the handler's established() treats a session as that address as proof, but this method still runs the takeover first.

Suggested change
if user and self.verification and not self.is_verified(email):
signed_in_as = (as_user or "").strip().lower() == email
if user and self.verification and not self.is_verified(email) \
and not signed_in_as:

The existing elif user: branch then handles the signed-in case. That branch already checks for a duplicate membership and calls join_org.

Failure scenario: bob@acme.com registers and stays unverified. Bob, signed in, opens an acme team link and types his own address. invite.html:51-55 tells him to leave the password blank. established() returns True, so server.py:2390 skips the length check. This method raises "choose a password", so bob cannot join with a blank password. If bob types a password such as "a", take_over sets a 1-character password, because the 8-character check was skipped. take_over also revokes bob's session. Then server.py:2404 redirects to / without a new cookie, so bob is signed out. Before fa0f55e the length check ran and line 2409 issued a fresh session. Add an HTTP-level test for the team-link path next to test_an_unverified_invitee_accepts_their_own_invite_without_a_password.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 596c9e2 with the suggested signed_in_as guard. HTTP-level test test_an_unverified_member_joins_through_a_team_link_without_a_password next to the single-use one: signed in and unverified, bob posts the team link with his address and a blank password, gets a 302 to /, is a member, keeps his session and password; a 1-char password typed afterwards is not installed.

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review — Blocking Issues: server/server.py:1260 — accept_team_invite still runs the takeover for a signed-in unverified user who uses a team link with their own address. fa0f55e makes established() return True in that case, so _handle_invite_accept skips the 8-character password check and the post-accept login. A blank password is refused with 'choose a password'. A typed password of any length takes over the account, revokes the user's own session, and redirects to / with no new cookie. Action Required: In accept_team_invite, take the plain-join branch when as_user equals email, as accept_invite does. Add an HTTP-level test for a signed-in unverified user accepting a team link with a blank password.

… own account

accept_team_invite ran the takeover whenever the row was unverified, even
for the person signed in as it. Since the handler now treats a matching
session as proof and skips the password check, that path would have set
an unchecked password and revoked the session. Mirror accept_invite: a
matching session is a plain join. HTTP-level test drives the team link
as that browser.
@eddietejeda
eddietejeda merged commit ea2a7a6 into main Oct 17, 2026
2 checks passed
@eddietejeda
eddietejeda deleted the tenant-isolation-hardening branch October 17, 2026 02:58
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.

1 participant