All gates green. Final report: **Phase 173 — final verification pass (all tasks already complete in `complete/`)** - Verified the full implementation is in the working tree: `docs_user`/`docs_token` settings + catalogue rows (task 01), `docs_repo` PUT 422 guard + `docs_token` write-only pin (task 02), `run_git(env)` + `push_document(user, token)` credential env (task 03), `settings_migrate.py` wired into `create_app()` (task 04), E2E suite (task 05). - **Completion criteria:** 1. GET `/api/admin/settings` lists `docs_user` (plain) + `docs_token` (secret, `value=null`), token never in any body — ✅ `test_docs_token_is_write_only_masked_end_to_end` + E2E sentinel-absent-from-DOM 2. PUT `docs_repo` with `://user:pass@` → 422 naming the fields; clean URL passes — ✅ integration + E2E UI-side 3. Token set → `GIT_CURL_USERNAME`/`GIT_CURL_PASSWORD`/`GIT_TERMINAL_PROMPT=0` on every git step; empty → `env=None` — ✅ mocked-git unit tests 4. Legacy env split once at startup, idempotent, one INFO line, `.env` untouched — ✅ `test_settings_migrate.py` (4 shapes) + `test_docs_push_migrate.py` 5. Full suite green, coverage >90%, lint clean — ✅ 6. E2E isolated: `test_docs_push_creds.py` 4/4; regression set (panel + layout + response_to_docs) 21/21 — ✅ 7. Commit + phase move — harness's job (per instructions, not mine). 8. README/AGENTS consistent — ✅ (below) - Tests/lint: `uv run pytest` exit 0 (4112 passed) · `uv run pytest --cov=app --cov-report=term-missing` TOTAL 99% · `uv run ruff check .` clean · `uv run pyright` 0 errors - Docs: README.md (incl. Docs-push config section, guard/migration paragraph, `.env.example` block) and AGENTS.md verified consistent in full; fixed two stale docstrings in `app/config.py` + `app/core/settings_catalog.py` (`is_set` → the API's actual `has_value` field). - No live-infrastructure changes (no live step in this phase). No next pending phase — `todo/` held only 173.
204 lines
7.7 KiB
Python
204 lines
7.7 KiB
Python
"""Integration: the phase-173 (task 04, C3) legacy-credentials
|
|
migration, end-to-end against the real ``app_settings`` table.
|
|
|
|
The shape (the 00_phase / task-04 pins):
|
|
|
|
* seed env ``BOR_DOCS_REPO=https://u:pass@host/r.git`` (no DB rows)
|
|
→ run the migration (the direct call — the ``create_app()`` wiring
|
|
is covered by the unit seam in ``test_main.py``) → the migration
|
|
returns ``True`` and the three rows land in ``app_settings``
|
|
(clean repo / ``docs_user`` / ``docs_token``);
|
|
* DB rows SHADOW the env — the effective settings resolve the clean
|
|
URL + the credentials FROM the DB (the env value still embeds the
|
|
credentials, ``.env`` is never rewritten: the process env is
|
|
untouched by the migration);
|
|
* idempotent — a second run is a no-op (``False``, the rows
|
|
byte-identical);
|
|
* the settings API reports the clean ``docs_repo`` (``source="db"``)
|
|
and the ``docs_token`` masked (``secret=true``, ``value=null``,
|
|
``has_value=true``) with the token text NOWHERE in the raw body —
|
|
the leak is gone from the page.
|
|
|
|
The module's teardown leaves NO row behind (the ``test_admin_settings_
|
|
api`` discipline — a surviving row would retune every later suite
|
|
through the overlay).
|
|
|
|
Real Postgres (``podman compose up -d db``); no LLM involved.
|
|
|
|
Requires: podman compose up -d db
|
|
"""
|
|
from __future__ import annotations
|
|
|
|
import os
|
|
from collections.abc import Iterator
|
|
|
|
import pytest
|
|
from fastapi.testclient import TestClient
|
|
from sqlalchemy import text
|
|
from sqlalchemy.orm import Session
|
|
|
|
from app.api import chat as chat_api
|
|
from app.config import get_settings, invalidate_settings
|
|
from app.core import settings_overlay
|
|
from app.core.settings_migrate import migrate_legacy_docs_creds
|
|
from app.core.settings_overlay import register_invalidator, resolve_settings
|
|
from app.main import app as fastapi_app
|
|
from tests.conftest import ADMIN_PASSWORD
|
|
|
|
API = "/api/admin/settings"
|
|
|
|
#: The legacy value the migration splits (the production shape — the
|
|
#: token is distinctive so the "absent from the raw body" pin cannot
|
|
#: false-positive on a catalogue label).
|
|
_LEGACY = "https://u:tok-legacy-123@host/r.git"
|
|
|
|
|
|
@pytest.fixture(autouse=True)
|
|
def clean_state(
|
|
db: Session, monkeypatch: pytest.MonkeyPatch
|
|
) -> Iterator[None]:
|
|
"""A clean ``app_settings`` table + the overlay switch ON + a
|
|
fresh overlay/hook slate (the ``test_admin_settings_api`` idiom:
|
|
reset the process-wide hook list and re-wire what
|
|
``create_app()`` wires). Teardown deletes the rows + invalidates
|
|
+ clears the cache — NO row may survive this module (a surviving
|
|
row would retune every later suite through the overlay)."""
|
|
settings_overlay.reset_state_for_tests()
|
|
register_invalidator(chat_api.reset_runtime_caches)
|
|
db.execute(text("DELETE FROM app_settings"))
|
|
db.commit()
|
|
monkeypatch.setenv("BOR_SETTINGS_DB", "1")
|
|
monkeypatch.setenv("BOR_DOCS_REPO", "")
|
|
invalidate_settings()
|
|
get_settings.cache_clear()
|
|
yield
|
|
db.execute(text("DELETE FROM app_settings"))
|
|
db.commit()
|
|
invalidate_settings()
|
|
get_settings.cache_clear()
|
|
settings_overlay.reset_state_for_tests()
|
|
|
|
|
|
def _table(db: Session) -> dict[str, str]:
|
|
"""The whole ``app_settings`` table (the display fact, never a
|
|
query surface — one SELECT)."""
|
|
return {
|
|
key: value
|
|
for key, value in db.execute(
|
|
text("SELECT key, value FROM app_settings")
|
|
)
|
|
}
|
|
|
|
|
|
def _admin_client() -> TestClient:
|
|
"""A fresh client signed in as the admin (the ``_admin_client``
|
|
pattern from ``test_ui_settings_api``)."""
|
|
client = TestClient(fastapi_app)
|
|
r = client.post("/api/login", json={"password": ADMIN_PASSWORD})
|
|
assert r.status_code == 204, f"admin login failed: {r.status_code}"
|
|
return client
|
|
|
|
|
|
def _item(body: dict, key: str) -> dict:
|
|
"""One catalogue item by key (the GET body's ``items`` list)."""
|
|
for item in body["items"]:
|
|
if item["key"] == key:
|
|
return item
|
|
raise AssertionError(f"catalogue item missing: {key}")
|
|
|
|
|
|
def test_legacy_env_is_split_into_db_rows(
|
|
db: Session, monkeypatch: pytest.MonkeyPatch
|
|
) -> None:
|
|
"""The end-to-end shape: seed env (no DB rows) → migration runs →
|
|
the three rows land, the effective settings resolve the clean URL
|
|
+ the credentials FROM THE DB (the rows shadow the env), the env
|
|
is untouched (``.env`` is never rewritten), and the second run is
|
|
a no-op (idempotent)."""
|
|
monkeypatch.setenv("BOR_DOCS_REPO", _LEGACY)
|
|
assert _table(db) == {} # no DB rows (the fixture's clean slate)
|
|
|
|
assert migrate_legacy_docs_creds() is True
|
|
|
|
# the three rows — the clean repo + user + token, and NOTHING
|
|
# else (the leak class dies: the repo row no longer carries the
|
|
# credentials).
|
|
rows = _table(db)
|
|
assert rows == {
|
|
"docs_repo": "https://host/r.git",
|
|
"docs_user": "u",
|
|
"docs_token": "tok-legacy-123",
|
|
}
|
|
|
|
# the effective settings resolve FROM THE DB (invalidate first —
|
|
# the module import primed the cache before this module's rows):
|
|
# the clean URL shadows the credentials-bearing env value.
|
|
invalidate_settings()
|
|
effective = resolve_settings()
|
|
assert effective.docs_repo == "https://host/r.git"
|
|
assert effective.docs_user == "u"
|
|
assert effective.docs_token == "tok-legacy-123"
|
|
|
|
# the env is untouched — the migration never rewrites the env
|
|
# (``.env`` stays the base; the DB rows shadow it).
|
|
assert os.environ["BOR_DOCS_REPO"] == _LEGACY
|
|
|
|
# idempotent: the ``docs_token`` row is present now — a second
|
|
# run is a no-op, the rows byte-identical.
|
|
assert migrate_legacy_docs_creds() is False
|
|
assert _table(db) == rows
|
|
|
|
|
|
def test_migrated_values_are_masked_on_the_settings_api(
|
|
db: Session, monkeypatch: pytest.MonkeyPatch
|
|
) -> None:
|
|
"""Completion criterion (the leak is gone from the page): after a
|
|
(b)-shape migration the settings API reports the clean
|
|
``docs_repo`` (``source="db"``) + ``docs_token``
|
|
``secret=true`` / ``value=null`` / ``has_value=true`` (masked)
|
|
with the token text NOWHERE in the raw response body."""
|
|
monkeypatch.setenv("BOR_DOCS_REPO", _LEGACY)
|
|
assert migrate_legacy_docs_creds() is True
|
|
invalidate_settings()
|
|
|
|
admin = _admin_client()
|
|
r = admin.get(API)
|
|
assert r.status_code == 200, r.text
|
|
|
|
repo = _item(r.json(), "docs_repo")
|
|
assert repo["value"] == "https://host/r.git" # the clean URL
|
|
assert repo["source"] == "db" # the DB row is in effect
|
|
assert repo["db_value"] == "https://host/r.git"
|
|
|
|
token = _item(r.json(), "docs_token")
|
|
assert token["secret"] is True
|
|
assert token["masked"] is True
|
|
assert token["value"] is None # write-only: never the value
|
|
assert token["db_value"] is None # a masked row never exposes text
|
|
assert token["has_value"] is True # a value IS in effect
|
|
assert token["has_override"] is True # the DB row
|
|
assert token["source"] == "db"
|
|
|
|
user = _item(r.json(), "docs_user")
|
|
assert user["value"] == "u"
|
|
assert user["source"] == "db"
|
|
|
|
# the core pin: the token text is NOWHERE in the raw body (the
|
|
# leak class this phase kills — asserted on response.text, not
|
|
# the parsed model; the value is distinctive so the substring
|
|
# check cannot false-positive on a catalogue label).
|
|
assert "tok-legacy-123" not in r.text
|
|
|
|
|
|
def test_migration_is_a_noop_for_a_clean_env(
|
|
db: Session, monkeypatch: pytest.MonkeyPatch
|
|
) -> None:
|
|
"""The no-op shape end-to-end: a clean env URL (the pre-phase
|
|
state) → ``False``, the table stays empty (nothing half-
|
|
migrates, nothing written)."""
|
|
monkeypatch.setenv("BOR_DOCS_REPO", "https://host/r.git")
|
|
assert migrate_legacy_docs_creds() is False
|
|
assert _table(db) == {}
|
|
invalidate_settings()
|
|
assert resolve_settings().docs_repo == "https://host/r.git"
|