Hacktoberfest 2026: los issues que los mantenedores marcaron para octubre, abiertos y aptos para principiantes. Explorar issues de Hacktoberfest

:bug: eventing: tenancy key derives from the GitHub login, so a released login inherits the previous holder's tenant

Abierto
#898 0 comentarios 0 reacciones 0 asignados Ver en GitHub

Los mantenedores suelen responder en 8 días

Nadie ha tomado este issue todavía.

Evaluación

Dificultad
4/5
Tiempo estimado
3-5 días
Aptitud para principiantes
68/100
Tipo de issue
Error
Claridad
Bien especificado
Estado de actividad
Activo
Stack tecnológico
python
Área
backend, security

Línea de trabajo

Start by reading eventbridge/ghauth.py (fetch_login), eventbridge/auth.py:104, shared/tenancy.py (userkey, _canonicalise, _slug) and eventbridge/registry.py (parse, resolve_caller) to see how the tenancy key is derived and where the recorded key is authoritative. Reproduce both failure snippets from the issue against those pure functions. Done means the key hashes the numeric GitHub id while login keeps feeding the readable slug and ce_submitter audit label, the allow-list/login vs tenancy-key case agreement test in the test suite is rewritten to state the new invariant, and existing registry entries with recorded keys still start (migration path documented in the docstring).

Escrito por el modelo de indexación a partir del texto del issue.

Descripción

Summary

userkey — the tenancy key that names a user's Kafka topics, their SQLite store and their
ntfy topic — is derived from the GitHub login, which is a renameable display name.
GitHub's durable identifier is the numeric id, and it is already in the response we
parse. Two consequences, one wasteful and one a cross-tenant leak.

Where

eventbridge/ghauth.py:161 reads the login and nothing else:

login = body.get("login")
if not isinstance(login, str) or not login:
    return None, "GitHub returned no login for this token"
return login, None

That string is the userid for the single production derivation point,
eventbridge/auth.py:104:

key = tenancy.userkey(issuer or ISSUER_STATIC, userid)

and shared/tenancy.userkey hashes f"{issuer}\x1f{canon}" where canon is
login.lower() (_canonicalise, github branch).

Failure 1 — a rename moves a user to an empty tenant

$ python3 -c "from shared import tenancy; print(tenancy.userkey('github','alice'))"
gh-alice-ed66acf0
$ python3 -c "from shared import tenancy; print(tenancy.userkey('github','alice-new'))"
gh-alice-new-a4d6e496

Different key, so a different {prefix}-{userkey}-requests topic, a different
sessions.sqlite under the per-user store root, and a different derived ntfy topic. The
user's history does not move with them — it reads as data loss, which is exactly the
symptom DESIGN_PHASE3.md §2.5 and registry.parse's startup refusal were written to
prevent for a derivation change. A rename produces the same outcome through an input
change, which neither guards.

Worth noting what partially saves us today: a user listed in the registry with a recorded
userkey makes this a startup refusal rather than silent breakage
(eventbridge/registry.py:161-171), because the recorded and computed values now
disagree. That is the good failure. But it is a refusal to start, not a migration — and it
only fires for users whose entry records a key. EB_ALLOWED_USERS (Phase 2's path, still
supported) records nothing, so there the rename is silent.

Failure 2 — a released login inherits the previous holder's tenant

This is the one that matters. GitHub frees a login when its owner renames or deletes
the account, and anyone may then claim it.

$ python3 -c "
from shared import tenancy
a = tenancy.userkey('github','alice')   # person 1
b = tenancy.userkey('github','alice')   # person 2, who later claimed the freed login
print(a); print(b); print('SAME TENANT:', a==b)"
gh-alice-ed66acf0
gh-alice-ed66acf0
SAME TENANT: True

So person 2 authenticates as alice, derives gh-alice-ed66acf0, and is routed to
person 1's store, topics and ntfy topic — reading their prompts, their agents' replies and
their notification stream. registry.parse's collision check does not fire, because from
its point of view there is one identity github/alice, listed once.

This defeats the property tenancy.userkey's own docstring claims:

The digest is the security control, not decoration. [...] two distinct canonical
identities cannot produce one key no matter how the slug collapses them.

The digest does hold up — it is collision-free over its input. The defect is that the
input does not distinguish two distinct humans, so "canonical identity" is doing less work
than the sentence implies. DESIGN_PHASE3.md §2.2 calls a GitHub login "already almost a
valid key" and considers only case-sensitivity; mutability is not discussed anywhere in
the phase documents.

Why the login is the wrong identifier, per GitHub

GitHub's own REST documentation says so, under GET /user/{account_id} — that endpoint

takes their durable user ID instead of their login, which can change over time.

And id is already in the response fetch_login parses — no extra API call, no extra
scope:

$ gh api user -q '{login,id,node_id}'
id=6954688  login=mrsabath  node_id=MDQ6VXNlcjY5NTQ2ODg=

Suggested fix

Derive from the id; keep the login for readability and audit only.

  1. fetch_login returns (login, id, error) — or a small frozen GhUser. The name
    fetch_login becomes a misnomer worth renaming.
  2. userkey("github", str(id)) becomes the key; the login feeds _slug so the key stays
    readable (gh-mrsabath-<digest over 6954688>). That keeps §2.3's "readable in
    kubectl get kafkatopics" property while making the hashed input durable. It does mean
    the slug can drift from the digest after a rename, which is cosmetic — but it should be
    stated in the docstring rather than discovered.
  3. ce_submitter should keep carrying the login: it is the human-meaningful audit
    label, and DESIGN_PHASE2.md §2.6 is about what it is worth, not about tenancy.
    Consider carrying the id as a separate attribute if attribution needs to survive a
    rename.
  4. ghauth.is_allowed still compares logins, which is right for an operator-written
    allow-list — but then the allow-list and the tenancy key no longer share an input, and
    _canonicalise's docstring explicitly requires they agree on case. That invariant
    needs restating rather than deleting: they are now answering different questions (may
    this person submit?
    vs which tenant is this?), and the test pinning their agreement
    should be rewritten to say so.

Migration

Not free, and this issue should not pretend otherwise. Any deployment with existing
per-user topics has keys derived from logins. Two cases:

  • Pre-GA, no real tenants. Change the derivation and accept that multi mode is not
    yet deployed anywhere. EB_TENANCY_MODE still defaults to single and §9's steps 5+
    are unimplemented, so this is likely the true state — which makes now the cheap
    moment to fix it.
  • With live tenants. The registry's recorded userkey is the migration seam: record
    the id in each entry, keep the recorded key authoritative (resolve_caller already
    prefers it — "the registry's recorded key is authoritative when present"), and derive
    from the id only for new signups.

Scope note

This is a Phase 3 defect, not a Phase 4 one. Filed separately from #897 (the Phase 4
design) rather than folded into it, per DESIGN_PHASE2.md §8.7: documenting a bug
honestly is not the same as accepting it, and a finding belongs with the code it
describes. #897's enrollment flow derives the same key, so it inherits this.

Verification

Both snippets above were run against eventing/ at dc1d432 with
shared/tenancy.py, eventbridge/ghauth.py and eventbridge/auth.py unmodified, and the
gh api user output is live. No cluster was involved — these are pure functions, which is
DESIGN_PHASE2.md §8.6's "cheap version" of running it.

Lenguaje dominante
Python
Estrellas
11
Forks
42
Merge medio
4 d 4 h
PR fusionados (30 d)
39

Preparar el entorno

Primeros pasos

  1. Lee el issue completo y luego la guía de contribución del proyecto.
  2. Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
  3. Haz un fork del repositorio y trabaja en una rama.
  4. Abre un pull request que haga referencia al número del issue.

Más de rossoctl/examples

Todos los issues de rossoctl/examples

Issues similares

Más issues de Python

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.