diff --git a/backend/tests/test_memory_rewrite.py b/backend/tests/test_memory_rewrite.py index 45f3e98..9790093 100644 --- a/backend/tests/test_memory_rewrite.py +++ b/backend/tests/test_memory_rewrite.py @@ -243,6 +243,23 @@ def test_the_rewrite_prompt_is_the_one_the_app_sends(db): # --------------------------------------------------------------------- the tool +def test_the_database_line_carries_no_password(): + """The report names the database it is about to rewrite. That line ends up + in a console, a screenshot or a pasted bug report.""" + shown = rewrite_memories.safe_dsn( + "postgresql://parth:hunter2@ep-cool-frost.us-east-1.aws.neon.tech/aidnd" + "?sslmode=require") + assert "hunter2" not in shown + assert "sslmode" not in shown # a password can be passed there too + assert shown == ("postgresql://parth@ep-cool-frost.us-east-1.aws.neon.tech" + "/aidnd") + + +def test_an_unparseable_database_url_shows_nothing_at_all(): + assert rewrite_memories.safe_dsn("not-a-url") == "(configured)" + + + def test_without_write_nothing_changes(db, monkeypatch): adventure = make_adventure(db) first, _ = fill_bank(db, adventure) diff --git a/backend/tools/rewrite_memories.py b/backend/tools/rewrite_memories.py index 76d41d5..f30faeb 100644 --- a/backend/tools/rewrite_memories.py +++ b/backend/tools/rewrite_memories.py @@ -93,9 +93,29 @@ import asyncio import sys from pathlib import Path +from urllib.parse import urlsplit + from sqlalchemy import func, inspect as sa_inspect, select +def safe_dsn(url: str) -> str: + """A connection string with the credentials taken out. + + The report says which database it is about to rewrite, which is worth + printing. The password in a Neon URL is not: this output goes to a console, + a screenshot, or a pasted bug report, and the operator has no way to know + the line carried a credential until it is somewhere else. + """ + parsed = urlsplit(url) + if not parsed.hostname: + return "(configured)" + who = f"{parsed.username}@" if parsed.username else "" + port = f":{parsed.port}" if parsed.port else "" + # The query string is dropped whole. `sslmode` is the only part anyone + # wants to see, and some drivers accept a password there too. + return f"{parsed.scheme}://{who}{parsed.hostname}{port}{parsed.path}" + + def words(text: str) -> int: return len(text.split()) @@ -111,7 +131,7 @@ async def main(args) -> int: from app.providers import OpenAICompatibleProvider, ProviderError db = SessionLocal() - print(f"database: {DATABASE_URL or DB_PATH}") + print(f"database: {safe_dsn(DATABASE_URL) if DATABASE_URL else DB_PATH}") if not sa_inspect(db.get_bind()).has_table(models.Adventure.__tablename__): # A mistyped path creates an empty SQLite file rather than failing, so # say what is wrong instead of raising "no such table: adventures". diff --git a/plan/18-persona-and-memory-quality.md b/plan/18-persona-and-memory-quality.md index 0e54b28..5ba1bed 100644 --- a/plan/18-persona-and-memory-quality.md +++ b/plan/18-persona-and-memory-quality.md @@ -4,7 +4,7 @@ Two changes, in order. Phase 1 gives the adventure a persona. Phase 2 uses it, along with the cast, to fix the memories. Phase 1 is worth shipping on its own; Phase 2 depends on it and is much smaller once it lands. -**Both phases are built and green (629 backend tests). Phase 1 was driven in a +**Both phases are built and green (631 backend tests). Phase 1 was driven in a browser (21/21 checks). Phase 2 was run end to end against a real model, as a controlled A/B on one story — see "Run with a real model". A bank written under the old prompt can be rewritten in place — see the last section.** diff --git a/plan/STATUS.md b/plan/STATUS.md index 0a5bb1f..792ed03 100644 --- a/plan/STATUS.md +++ b/plan/STATUS.md @@ -81,7 +81,7 @@ needed; nothing requires reading a row of anyone's story. **`plan/18-persona-and-memory-quality.md` is the writeup. Both changes are on `main`** — the note here that said they were sitting unmerged on `claude/ai-dnd-memories-summarization-3muo98` is out of date; `main` is at `71b24b6`, -the tip of that work. Both green at 629 tests, plus the backfill below. +the tip of that work. Both green at 631 tests, plus the backfill below. **The protagonist now has a name.** An adventure carries `persona_name`, `persona_pronouns` and `persona_desc` (migrations 74-76), and the player's stat block