Skip to content

A profile with no configuration connects to a local PostgreSQL it was never configured for #704

Open

Description

@cdeust

Summary

A fresh profile with no configuration connects to a PostgreSQL at 127.0.0.1:5432/cortex that nobody configured for it, reads it, and starts background consolidation against it. On a machine that happens to run a local cortex database, an unrelated session (or a test, or an audit) reaches real user data.

Evidence

Observed on 2026-10-10 by an independent audit that believed its profile was isolated. No backend marker existed in the isolated home, so the SessionStart hook used its default URL, connected to the owner's live database, read it, printed a banner built from it and spawned two consolidate_background processes plus a pipeline reanalysis and a groomer coordinator start (the audit stopped them by pid within about 14 s and 73 s; no row was added to the consolidation log).

  • mcp_server/hooks/session_start.py line 48: _DATABASE_URL = os.environ.get("DATABASE_URL", "postgresql://127.0.0.1:5432/cortex").
  • mcp_server/infrastructure/memory_config.py line 50: DATABASE_URL: str = "postgresql://127.0.0.1:5432/cortex".
  • scripts/install-plugin.sh (around line 121) silently selects PostgreSQL when it finds a local cortex database ("never downgrade"), so the same implicit choice exists at install time. Read in the script, not run against a live PostgreSQL.

Expected

PostgreSQL is used only when the user sets a database URL or when a marker says so, never because a default URL happens to answer. No default DATABASE_URL in code. A hook that has no marker and no URL uses SQLite and does not open any network connection. A test that runs every hook entry point with port 5432 listening on a throwaway server and asserts that nothing connects to it. Related: issue about the install path that never writes the marker.

Activity

  1. cdeust commented on Nov 4, 2026

    @cdeust
    OwnerAuthor

    Correctif proposé dans #865 (commit 1ba96df) pour le reliquat bibliothèque que #804 avait laissé hors périmètre.

    Cause racine : MemorySettings.DATABASE_URL valait par défaut postgresql://127.0.0.1:5432/cortex. En cli, auto devenait donc PostgreSQL sur cette URL, et cortex-migrate comme la connexion brute d'agent-briefing s'y connectaient sans configuration. Le défaut est maintenant vide. auto sans URL nommée donne SQLite, sans aucune tentative de connexion. Un backend postgresql sans URL lève une erreur. _get_database_url refuse une conninfo vide.

    Tests : 10 nouveaux tests, dont 9 échouent sur main 1de07f9 et passent avec le correctif. Suite complète : 10904 passés et 4 échecs sans rapport avec la PR (3 dus à torch, non installable ici ; 1 test wiki déjà rouge sur main sous root).

    Tranche restante, décision du propriétaire : scripts/install-plugin.sh (l. 20-21 et 87) choisit encore PostgreSQL à l'installation quand une base cortex locale répond (« never downgrade »). Le retirer changerait les mises à jour des installations PostgreSQL existantes. La PR référence donc l'issue avec Refs #704, sans Closes.


    Generated by Claude Code

  2. cdeust commented on Nov 7, 2026

    @cdeust
    OwnerAuthor

    Mise en attente : la seule tranche restante demande une décision du propriétaire.

    #937 (fusionnée, e47a499) a retiré l'URL PostgreSQL par défaut du code bibliothèque. auto sans URL donne maintenant SQLite, et postgresql sans URL lève une erreur. Il reste scripts/install-plugin.sh (lignes 20–21 et 87 selon #937) : à l'installation, il choisit encore PostgreSQL s'il trouve une base cortex locale joignable (« never downgrade »).

    Question à trancher : à l'installation ou à la mise à jour, sans marqueur ni URL explicite, faut-il

    • (a) ne plus jamais choisir PostgreSQL implicitement (SQLite par défaut, PostgreSQL seulement sur DATABASE_URL ou marqueur), ce qui change la mise à jour des installations PostgreSQL existantes sans marqueur ;
    • (b) garder la détection pour les mises à jour, mais écrire alors un marqueur explicite et l'annoncer à l'utilisateur ;
    • (c) autre chose ?

    La réponse détermine aussi si le test demandé par l'issue (chaque point d'entrée de hook face à un port 5432 qui écoute) doit couvrir l'installateur.


    Generated by Claude Code

  3. 63 remaining items

  4. cdeust commented on Nov 14, 2026

    @cdeust
    OwnerAuthor

    CI autopilot : #1049 fusionnée (revue 1 de 3 : approve, aucun Blocker ; Codex : aucun constat en revue de code ni en revue de sécurité). Notes « no action before merge » à livrer dans un prochain passage, avec les notes 1 et 2 de la revue de #1048 :

    1. memory_store.py _named_url teste candidate.strip() mais renvoie la valeur avec ses espaces. L'installateur et settle_backend la retirent : soit retirer les espaces, soit corriger la docstring.
    2. mcp_server/hooks/agent_briefing_query.py:34 garde os.environ.get("DATABASE_URL") or settings.DATABASE_URL : une valeur faite de blancs y atteint encore pg_connect.
    3. _database_url_is_explicit (pinné par test_construct_store_selection.py) compte encore un argument blanc ou une clé DATABASE_URL blanche comme explicite. La revue n'a trouvé aucun effet atteignable aujourd'hui, mais c'est incohérent avec la règle « une valeur blanche ne nomme rien ».

    Generated by Claude Code

  5. cdeust commented on Nov 14, 2026

    @cdeust
    OwnerAuthor

    CI autopilot : notes 1 à 3 de la revue de #1049 (et 1-2 de #1048) livrées dans #1050 (34b1bc8, Refs #704).

    La règle « une valeur faite de blancs ne nomme rien » est désormais écrite une seule fois, dans memory_config.named_database_url, qui renvoie le premier candidat non blanc, sans ses espaces. _named_url, _database_url_is_explicit et agent_briefing_query._connect l'utilisent ; avant ce changement, le hook de briefing passait un DATABASE_URL blanc à pg_connect. 9 nouveaux tests échouent sans le correctif et passent avec. Suites unit et regression : voir le corps de la PR, avec les 11 échecs d'environnement, identiques sur main.

    Reste hors périmètre : groomer_coordinator.py:60 (identité de verrou, pas une connexion).


    Generated by Claude Code

  6. cdeust commented on Nov 14, 2026

    @cdeust
    OwnerAuthor

    CI autopilot : #1050 approuvée (revue 1 de 3, 34b1bc8, aucun Blocker ; Codex : aucun constat, ni en revue de code ni en revue de sécurité). Je n'ai rien poussé sur la PR, conformément au contrat CI (pas de commentaire après un approve). Les notes « no action before merge » sont à livrer dans une PR de suivi, après la fusion :

    1. pg_store_ddl._get_database_url (l. 62-64) ignore aussi un placeholder de manifeste non développé (${…}), ce que named_database_url ne fait pas. Il faut aligner les deux règles, ou corriger le CHANGELOG, qui parle de « règle unique ». auto_recall.py:65 et session_start_common.py:15 lisent encore DATABASE_URL brut.
    2. Le DSN blanc du hook n'est atteignable qu'en appel direct de _connect : settle_backend supprime d'abord un DATABASE_URL blanc. Il faut préciser le CHANGELOG.
    3. Ajouter le test du hook avec le backend auto et des URL toutes blanches (garde agent_briefing_query.py:40).

    Generated by Claude Code

  7. cdeust commented on Nov 15, 2026

    @cdeust
    OwnerAuthor

    CI autopilot : notes 1 à 3 de la revue de #1050 livrées dans #1051 (49ce25a, Refs #704).

    La règle « une valeur nomme une base » existait en deux versions : cortex-migrate (pg_store_ddl._get_database_url) ignorait aussi un placeholder de manifeste non développé (${user_config.database_url}), contrairement au store et au hook de briefing ; auto_recall et session_start_common lisaient DATABASE_URL brut. La règle vit maintenant une seule fois dans mcp_server/shared/database_url.py (stdlib seule) : ni blanche, ni ${. Le CHANGELOG précise que le DSN blanc n'atteignait libpq qu'en appel direct de _connect, et un test couvre le hook en backend auto avec des URL toutes blanches.

    Suites unit et regression : voir le corps de la PR (mêmes 11 échecs d'environnement que sur main). Hors périmètre : scripts/setup_db.py:53 (copie de la règle côté scripts), groomer_coordinator.py:60 (identité de verrou).


    Generated by Claude Code

  8. cdeust commented on Nov 15, 2026

    @cdeust
    OwnerAuthor

    CI autopilot : #1051 approuvée (revue 3 de 3, 62c7d47, aucun Blocker ; Codex : aucun constat en revue de code sur 62c7d47). Les Blockers des revues 1 et 2 (placeholder ${…} compté comme URL par la décision de backend, avec ou sans marqueur) sont corrigés dans 5b69584 et 62c7d47. Je n'ai rien poussé ni commenté sur la PR depuis l'approve (contrat CI, règle 6). Notes « no action before merge », à livrer dans une PR de suivi après la fusion :

    1. scripts/setup_db.py:56 écarte encore toute valeur contenant ${ ; docs/module-inventory.md:44 et la docstring de l'étape 3 (backend_marker.py:16) décrivent l'ancienne règle.
    2. launcher_backend.map_database_option (scripts/launcher_backend.py:18) ne teste que .strip() : un DATABASE_URL placeholder empêche de reporter un vrai CLAUDE_PLUGIN_OPTION_DATABASE_URL.
    3. Un CORTEX_MEMORY_STORE_BACKEND non développé (${user_config.store_backend}) compte encore comme backend explicite (code inchangé).
      Notes de la revue 2 encore ouvertes : effective_backend (doctor) et les contrôles de scripts/launcher_pg_select.py / launcher_pg_provision.py lisent l'environnement brut ; une valeur écartée ne produit aucune ligne de log.

    Generated by Claude Code

  9. cdeust commented on Nov 15, 2026

    @cdeust
    OwnerAuthor

    CI autopilot : les notes 1 à 3 de la revue de #1051 sont dans #1052 (f69cd5b, Refs #704).

    • scripts/setup_db.py : un ${ dans un DSN (mot de passe littéral) n'est plus remplacé par le défaut local ; seul un jeton ${name} entier vaut « non défini ».
    • launcher_backend.map_database_option : un DATABASE_URL resté ${user_config.database_url} ne bloque plus le report d'un vrai CLAUDE_PLUGIN_OPTION_DATABASE_URL.
    • CORTEX_MEMORY_STORE_BACKEND resté ${user_config.store_backend} (manifest.json) ne compte plus comme backend explicite ; apply_backend_resolution le retire de l'environnement.

    Tests : 7 nouveaux tests échouent sur main et passent sur la branche ; suite différentielle shared/infrastructure/scripts/hooks : mêmes échecs des deux côtés (dépendances absentes du venv), +22 réussis. Non lancés : unit et regression de ci.json (download-r2.pytorch.org refusé par le proxy de la session).

    Tranche restante (notes de la revue 2 de #1051) : effective_backend côté doctor et les contrôles de launcher_pg_select.py / launcher_pg_provision.py lisent l'environnement brut ; une valeur écartée ne produit aucune ligne de log.


    Generated by Claude Code

  10. cdeust commented on Nov 15, 2026

    @cdeust
    OwnerAuthor

    CI autopilot : #1052 approuvée (revue 1 de 3, f69cd5b, aucun Blocker ; Codex sécurité : aucun constat). Je n'ai rien poussé ni commenté sur la PR depuis l'approve (contrat CI, règles 5 et 6). À livrer dans une PR de suivi après la fusion :

    1. Codex P2 (confirmé, reproduit) : python scripts/setup_db.py lancé depuis un checkout sans PYTHONPATH lève ModuleNotFoundError: mcp_server avant le JSON (même constat que la note 3 de la revue Claude). Correctif prêt et testé localement (15 tests OK ; le script rend de nouveau needs_install), non poussé :
    diff --git a/scripts/setup_db.py b/scripts/setup_db.py
    index 505be487..3fc6447a 100644
    --- a/scripts/setup_db.py
    +++ b/scripts/setup_db.py
    @@ -20,6 +20,7 @@ import re
     import shutil
     import subprocess
     import sys
    +from pathlib import Path
     
     
     def _safe_ident(name: str) -> str:
    @@ -54,7 +55,12 @@ def _get_database_url() -> str:
         by the rule the store and the hooks apply; a ``${`` inside a DSN (a
         literal password) is kept (issue #704, note 1 of the review of #1051).
         """
    -    from mcp_server.shared.database_url import named_database_url  # noqa: PLC0415 — both callers (setup.sh, setup.py) put the project on PYTHONPATH; stdlib-only module, so importing setup_db stays stdlib-only
    +    # Run directly (docs/deployment-scenarios.md), Python puts scripts/ on the
    +    # path, not the checkout: append it, so an installed package still wins.
    +    checkout = str(Path(__file__).resolve().parents[1])
    +    if checkout not in sys.path:
    +        sys.path.append(checkout)
    +    from mcp_server.shared.database_url import named_database_url  # noqa: PLC0415 — the checkout is on sys.path only from here; stdlib-only module, so importing setup_db stays stdlib-only
     
         return named_database_url(os.environ.get("DATABASE_URL")) or (
             "postgresql://127.0.0.1:5432/cortex"

    Test associé : lancer [sys.executable, "-I", scripts/setup_db.py] sans PYTHONPATH, avec DATABASE_URL vers 127.0.0.1:1 ; vérifier l'absence de ModuleNotFoundError et une dernière ligne JSON qui porte status.
    2. scripts/install-plugin.sh:119 et :154-157 (mapping de l'option et decision()), ainsi que scripts/setup.py:45,55, appliquent encore l'ancienne règle .strip(). La ligne du CHANGELOG « now holds outside the store and the hooks too » est à resserrer.
    3. Dans la docstring de backend_marker.py:20-23, le paragraphe coupe la liste numérotée entre les étapes 3 et 4. Il dit à tort que la valeur « est retirée » : seul apply_backend_resolution la retire, les trois autres fonctions l'ignorent.
    4. Toujours ouvert (revue 2 de #1051) : effective_backend (doctor), launcher_pg_select.py et launcher_pg_provision.py ; une valeur écartée ne produit aucune ligne de log.


    Generated by Claude Code

  11. cdeust commented on Nov 15, 2026

    @cdeust
    OwnerAuthor

    CI autopilot : #1052, revue 2 de 3 : approve (899aad3, aucun Blocker ; Codex code et sécurité : aucun constat sur 899aad3). Le push 899aad3 corrigeait un Blocker CI : la porte de mutation du job unit (run 43668782716) gardait deux mutants équivalents dans _explicit_backend, à cause d'un défaut "" redondant ; la logique « prendre et nettoyer » est désormais regroupée dans named_setting. Ce push contient aussi le correctif du P2 Codex (setup_db.py lancé seul). Rien poussé ni commenté depuis l'approve. À ajouter à la PR de suivi (en plus de mon commentaire précédent, points 2 à 4) :

    • tests_py/scripts/test_scripts_unexpanded_database_url.py:64-79 passe aussi sans le correctif en CI : l'installation editable charge mcp_server depuis site-packages. Le lancer avec -S et retirer le PYTHONPATH, inutile sous -I.
    • scripts/setup_db.py:58-63 : un mcp_server installé antérieur à fix(store): an unexpanded placeholder names no database in the store and the hooks (#704) #1051 (sans shared.database_url) masque le checkout ajouté en fin de chemin. La modification de sys.path dans un getter conditionne aussi les imports de :223 et :261 : la placer au niveau du module, sous le garde du script.
    • CHANGELOG.md et docs/module-inventory.md:44 doivent nommer named_setting. Le texte du noqa (« only from here ») affirme plus que la réalité.

    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions