**Phase 121 final verification pass — all green** (all 4 tasks already in `complete/`; verified, no defects found, no changes needed) - Verified implementation vs phase design: migration `0021` (reversible, round-tripped via `alembic downgrade base` + `upgrade head` → head `0021`), `GitSource.token` column, `normalize_credential`/`clone_url_for`/`sanitize_url`, clone callers switched (`sync.py`, `import_docs.py`), masked token fields in add form + editor, `extra="forbid"` output shapes - Tests: `uv run pytest` → 2662 passed, 0 failed (exit 0); `uv run pytest --cov=app --cov-report=term-missing` → TOTAL **99%** (≥90% gate) - Lint/types: `uv run ruff check .` → All checks passed; `uv run pyright` → 0 errors, 0 warnings - E2E in isolation: `uv run pytest tests/e2e/test_git_source_tokens.py -v --no-cov` → **4 passed** Completion criteria: 1. Private repo (UI add or pasted embedded-token URL) clones with injected token; token absent from every API response, page text, title attr, and full HTML — **PASS** (integration raw-JSON assertions + E2E `_assert_token_nowhere`) 2. Legacy embedded-token rows still clone from stored URL; output sanitized — **PASS** (`test_sync_legacy_row_clones_with_original_stored_url`, `test_get_masks_legacy_embedded_token_row`, env-fallback masking) 3. Public/local sources byte-identical — **PASS** (verbatim-URL + no-userinfo-unchanged tests) 4. pytest / coverage / ruff / pyright — **PASS** (see above) 5. Commit + phase move — harness responsibility; task files already in `complete/`, changes left in working tree (no commit made, per protocol) Notable: no deviations; DB left at head, functional. Next pending phase: **122_image_documents** (then 123_chat_image_questions).
430 lines
19 KiB
Python
430 lines
19 KiB
Python
"""Phase 121 story E2E (Playwright): private git sources — a token that
|
|
never reaches the UI or the API (LOCKED A2/A6).
|
|
|
|
Source: ``TODO.md:5`` — "Need a way to add private repos without exposing
|
|
the token in the UI (like when adding an https repo
|
|
``https://myuser:ghp_xxx@github.com/myuser/my-private-repo.git``)".
|
|
|
|
Run in isolation (DB must be up: ``podman compose up -d db``):
|
|
|
|
uv run pytest tests/e2e/test_git_source_tokens.py -v --no-cov
|
|
|
|
**No git, no network** — nothing in this suite triggers a sync (the
|
|
clone-time credential injection is pinned at the integration level with a
|
|
mocked ``clone_or_pull``); this suite drives the real Sources page and
|
|
the real admin-only CRUD API and asserts the credential contract on every
|
|
surface the owner can see: the rendered row, the page text, every
|
|
``title`` attribute, the page HTML, and the raw JSON of
|
|
``GET /api/git-sources`` (via the logged-in page context's request
|
|
client — the same cookie as the UI).
|
|
|
|
Per-module app env (the conftest pattern, module-scoped — as in
|
|
``test_git_sources_admin.py``): this story's app boots on its own port
|
|
(a same-port second uvicorn would die on bind and drive the wrong
|
|
server) with ``BOR_GIT_SOURCES`` pinned EMPTY (the phase-61 leak guard —
|
|
the operator's local gitignored ``.env`` carries the owner's real source
|
|
list, and process env ranks above the ``.env`` file; a leaked env list
|
|
would flip the empty-after-removal assertions) and ``BOR_SOURCES_DIR`` /
|
|
``BOR_UPLOAD_DIR`` pinned to fresh tmp dirs (the cleanup DELETE must
|
|
never aim a rmtree at the operator's real checkouts — the integration
|
|
``_settings`` pattern). The mock-calibrated lexical floor is pinned
|
|
like the conftest (the 0.30 mock threshold + the code-default 0.35 floor
|
|
violate the startup validator).
|
|
|
|
Contract under test (the phase's completion criteria, UI side):
|
|
|
|
* add a private repo through the Sources UI (bare URL + the masked
|
|
``#git-source-token`` field) → 201 → the row renders the BARE URL
|
|
(mono cell + the cell's ``title`` attribute), the token is absent from
|
|
``document.body.innerText``, every ``title`` attribute, the page HTML,
|
|
and the ``GET /api/git-sources`` raw JSON; the DB row stores the bare
|
|
URL + the token column (the DB is the trusted store); both inputs
|
|
clear and the button re-enables (§7.4 never-stale);
|
|
* pasting the old-style embedded-token URL (``user:token@``, no token
|
|
field — the TODO L5 shape) is accepted and normalized server-side
|
|
(LOCKED A6): stored bare + the token column populated, and the token
|
|
(and the username) is nowhere in the UI or the API;
|
|
* the per-row editor's ``#ignore-editor-token`` is ``type=password`` and
|
|
opens BLANK (a password is never echoed back — there is no token
|
|
field to prefill from); saving it blank sends a PATCH that OMITS the
|
|
key (the tri-state no-change), gets 200, and the row's stored token is
|
|
KEPT (DB assertion) — the row is intact;
|
|
* removing the source (the phase-69 confirmation modal) → 204, the list
|
|
is empty again, and the token is gone from every surface.
|
|
|
|
Test → mapping:
|
|
|
|
1. ``test_admin_add_private_repo_token_stays_out_of_ui_and_api``
|
|
2. ``test_admin_embedded_token_url_paste_is_normalized_and_hidden``
|
|
3. ``test_admin_editor_token_opens_blank_and_save_keeps_token``
|
|
4. ``test_admin_remove_private_repo_leaves_empty_list``
|
|
"""
|
|
from __future__ import annotations
|
|
|
|
import json
|
|
import os
|
|
import subprocess
|
|
import sys
|
|
import tempfile
|
|
from collections.abc import Iterator
|
|
from pathlib import Path
|
|
from typing import Any
|
|
|
|
import pytest
|
|
from playwright.sync_api import Page, expect
|
|
from sqlalchemy import select, text
|
|
|
|
from app.db import SessionLocal
|
|
from app.models import GitSource
|
|
from e2e.auth_helpers import login
|
|
from e2e.conftest import ADMIN_PASSWORD, SESSION_SECRET, USE_REAL_LLM, _wait_http
|
|
|
|
REPO = Path(__file__).resolve().parents[2]
|
|
|
|
# Phase 79 (task 04, full inventory): the conftest session app owns its
|
|
# port in a combined run — this module app binds its own port instead
|
|
# (env-overridable, as in test_git_sources_admin.py).
|
|
APP_PORT = int(os.environ.get("E2E_APP_PORT_GITTOKEN", "8146"))
|
|
APP_URL = f"http://127.0.0.1:{APP_PORT}"
|
|
|
|
GIT_SOURCES_URL = "/git-sources.html"
|
|
|
|
#: Deterministic fixtures: a private repo, a distinctive fake token (the
|
|
#: "nowhere" assertions key on this exact string), and the old-style
|
|
#: embedded-token paste of a SECOND repo (the TODO L5 shape).
|
|
PRIVATE_URL = "https://github.com/acme/e2e-private-token.git"
|
|
TOKEN = "ghp_e2esecrettoken0123456789"
|
|
LEGACY_URL = f"https://myuser:{TOKEN}@github.com/acme/e2e-legacy.git"
|
|
LEGACY_BARE_URL = "https://github.com/acme/e2e-legacy.git"
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Fixtures
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
@pytest.fixture(scope="module")
|
|
def app_server(mock_llm: int) -> Iterator[str]:
|
|
"""The real app under test — per-module env: ``BOR_GIT_SOURCES``
|
|
pinned EMPTY (the phase-61 leak guard — see the module docstring)
|
|
and ``BOR_SOURCES_DIR`` / ``BOR_UPLOAD_DIR`` pinned to fresh tmp
|
|
dirs (the cleanup DELETE never rmtrees the operator's real
|
|
checkouts). No git, no sync in this suite."""
|
|
env = dict(os.environ)
|
|
env.pop("DEBUGPY", None)
|
|
env["BOR_ENVIRONMENT"] = "e2e"
|
|
env["BOR_STATIC_DIR"] = str(REPO / "frontend")
|
|
env["BOR_LLM_BASE_URL"] = (
|
|
"https://aipi.reeseapps.com/v1"
|
|
if USE_REAL_LLM
|
|
else f"http://127.0.0.1:{mock_llm}/v1"
|
|
)
|
|
# Mock-calibrated gate (conftest pattern) — no chat turn is sent,
|
|
# but the app boots with the same env shape. The floor is pinned
|
|
# explicitly: the code default (0.35) violates the startup
|
|
# validator against the mock threshold (0.30) and would refuse the
|
|
# boot if the operator's .env did not carry a valid one.
|
|
env["BOR_RELEVANCE_THRESHOLD"] = "0.30"
|
|
env["BOR_LEXICAL_SUPPORT_FLOOR"] = "0.15"
|
|
env.setdefault(
|
|
"BOR_DATABASE_URL",
|
|
"postgresql+psycopg://reese:reese@localhost:5432/brain_of_reese",
|
|
)
|
|
# Phase 16: admin auth must be set or create_app() refuses to boot.
|
|
env["BOR_ADMIN_PASSWORD"] = ADMIN_PASSWORD
|
|
env["BOR_SESSION_SECRET"] = SESSION_SECRET
|
|
# The empty-table env fallback list — pinned EMPTY: the operator's
|
|
# local (gitignored) .env must not leak the owner's real sources
|
|
# into the empty-state assertions.
|
|
env["BOR_GIT_SOURCES"] = ""
|
|
# Fresh scratch dirs: the cleanup DELETE (test 4) must never aim a
|
|
# rmtree at the operator's real checkouts (the integration
|
|
# ``_settings`` pattern).
|
|
scratch = Path(tempfile.mkdtemp(prefix="bor-e2e-gittoken-"))
|
|
env["BOR_SOURCES_DIR"] = str(scratch / "sources")
|
|
env["BOR_UPLOAD_DIR"] = str(scratch / "uploads")
|
|
proc = subprocess.Popen(
|
|
[sys.executable, "-m", "uvicorn", "app.main:app",
|
|
"--host", "127.0.0.1", "--port", str(APP_PORT), "--log-level", "warning"],
|
|
cwd=REPO,
|
|
env=env,
|
|
)
|
|
try:
|
|
_wait_http(f"{APP_URL}/api/health")
|
|
yield APP_URL
|
|
finally:
|
|
proc.terminate()
|
|
try:
|
|
proc.wait(timeout=10)
|
|
except subprocess.TimeoutExpired:
|
|
proc.kill()
|
|
|
|
|
|
@pytest.fixture(scope="module")
|
|
def app_url(app_server: str) -> str:
|
|
return app_server
|
|
|
|
|
|
def _truncate_git_sources() -> None:
|
|
with SessionLocal() as db:
|
|
db.execute(text("TRUNCATE git_sources"))
|
|
db.commit()
|
|
|
|
|
|
@pytest.fixture(autouse=True)
|
|
def _clean_git_sources(db_ready: None) -> Iterator[None]:
|
|
"""Fresh ``git_sources`` table per test — this suite owns the table
|
|
(the E2E isolation pattern, as in ``test_git_sources_admin.py``);
|
|
emptied BOTH before and after every test (the suites share one
|
|
Postgres and run in isolation — a leftover row would flip another
|
|
suite's app from the env fallback to the DB list)."""
|
|
_truncate_git_sources()
|
|
yield
|
|
_truncate_git_sources()
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Helpers
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
def _seed_token_row(url: str, token: str) -> None:
|
|
"""Store a token row directly (deterministic; the page loads the
|
|
list on boot, so seed BEFORE navigating)."""
|
|
with SessionLocal() as db:
|
|
db.add(GitSource(url=url, token=token))
|
|
db.commit()
|
|
|
|
|
|
def _stored_row() -> tuple[str, str | None]:
|
|
"""The stored row's ``(url, token)`` — read INSIDE the session
|
|
(detached instances would defer-load on attribute access)."""
|
|
with SessionLocal() as db:
|
|
row = db.scalars(select(GitSource)).one()
|
|
return row.url, row.token
|
|
|
|
|
|
def _admin_git_sources_page(page: Page, app_url: str) -> None:
|
|
"""Real form login landing on the git sources page (admin settled:
|
|
Sign out visible, the manager revealed by the page module)."""
|
|
login(page, app_url, next=GIT_SOURCES_URL)
|
|
expect(page).to_have_url(app_url + GIT_SOURCES_URL, timeout=30_000)
|
|
expect(page.locator("#sign-out-btn")).to_be_visible(timeout=15_000)
|
|
expect(page.locator("#git-sources-gate")).to_be_hidden()
|
|
expect(page.locator("#git-sources-content")).to_be_visible()
|
|
|
|
|
|
def _assert_token_nowhere(page: Page, app_url: str, token: str) -> None:
|
|
"""The phase's credential contract (LOCKED A2): the token is absent
|
|
from EVERY surface the owner can see — the rendered body text,
|
|
every ``title`` attribute, the whole page HTML (every serialized
|
|
attribute value), and the raw JSON text of ``GET /api/git-sources``
|
|
(the page context's request client carries the logged-in cookie)."""
|
|
body_text = page.evaluate("() => document.body.innerText")
|
|
assert token not in body_text, "the token leaks into the page text"
|
|
titles = page.evaluate(
|
|
"() => [...document.querySelectorAll('[title]')].map((el) => el.getAttribute('title'))"
|
|
)
|
|
for t in titles:
|
|
assert t is None or token not in t, f"the token leaks into a title attribute: {t!r}"
|
|
assert token not in page.content(), "the token leaks into the page HTML"
|
|
r = page.request.get(f"{app_url}/api/git-sources")
|
|
assert r.status == 200, r.text
|
|
assert token not in r.text(), "the token leaks into GET /api/git-sources"
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# 1. Add: bare URL + masked token → the row is bare, the token is
|
|
# nowhere (page text, title attributes, HTML, API JSON)
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
def test_admin_add_private_repo_token_stays_out_of_ui_and_api(
|
|
page: Page, app_url: str, db_ready: None
|
|
) -> None:
|
|
page.set_default_timeout(30_000)
|
|
_admin_git_sources_page(page, app_url)
|
|
expect(page.locator("#git-sources-tbody tr")).to_have_count(0)
|
|
|
|
page.fill("#git-source-url", PRIVATE_URL)
|
|
page.fill("#git-source-token", TOKEN)
|
|
page.click("#git-source-add")
|
|
|
|
# 201 → the row lands rendering the BARE URL — the mono code cell
|
|
# AND the cell's hover title attribute are both bare.
|
|
row = page.locator("#git-sources-tbody tr", has_text=PRIVATE_URL)
|
|
expect(row).to_have_count(1, timeout=30_000)
|
|
expect(row.locator("td.git-source-url-cell code")).to_have_text(PRIVATE_URL)
|
|
expect(row.locator("td.git-source-url-cell")).to_have_attribute("title", PRIVATE_URL)
|
|
|
|
# Never-stale (§7.4): BOTH inputs clear (the credential is stored —
|
|
# write-only, never round-trips), the button re-enables with its
|
|
# idle label, no inline error.
|
|
expect(page.locator("#git-source-url")).to_have_value("")
|
|
expect(page.locator("#git-source-token")).to_have_value("")
|
|
expect(page.locator("#git-source-add")).to_be_enabled()
|
|
expect(page.locator("#git-source-add")).to_have_text("Add source")
|
|
expect(page.locator("#git-source-error")).to_be_hidden()
|
|
|
|
# The DB is the trusted store (LOCKED A2): bare URL + the token
|
|
# column — the token lives in exactly one place.
|
|
stored_url, stored_token = _stored_row()
|
|
assert stored_url == PRIVATE_URL
|
|
assert stored_token == TOKEN
|
|
|
|
_assert_token_nowhere(page, app_url, TOKEN)
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# 2. Paste the old-style embedded-token URL (TODO L5): normalized
|
|
# server-side (LOCKED A6), token + username nowhere
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
def test_admin_embedded_token_url_paste_is_normalized_and_hidden(
|
|
page: Page, app_url: str, db_ready: None
|
|
) -> None:
|
|
page.set_default_timeout(30_000)
|
|
_admin_git_sources_page(page, app_url)
|
|
expect(page.locator("#git-sources-tbody tr")).to_have_count(0)
|
|
|
|
# The exact TODO L5 paste: ``user:token@`` in the URL, the masked
|
|
# token field BLANK (the key is omitted from the POST).
|
|
page.fill("#git-source-url", LEGACY_URL)
|
|
page.click("#git-source-add")
|
|
|
|
# 201 → the row is the BARE URL (the userinfo was stripped on
|
|
# write), in the code cell AND the title attribute.
|
|
row = page.locator("#git-sources-tbody tr", has_text=LEGACY_BARE_URL)
|
|
expect(row).to_have_count(1, timeout=30_000)
|
|
expect(row.locator("td.git-source-url-cell code")).to_have_text(LEGACY_BARE_URL)
|
|
expect(row.locator("td.git-source-url-cell")).to_have_attribute("title", LEGACY_BARE_URL)
|
|
|
|
expect(page.locator("#git-source-error")).to_be_hidden()
|
|
# The paste is normalized, not rejected: stored bare + the embedded
|
|
# PASSWORD part in the token column (the username is an identifier,
|
|
# not the credential).
|
|
stored_url, stored_token = _stored_row()
|
|
assert stored_url == LEGACY_BARE_URL
|
|
assert stored_token == TOKEN
|
|
|
|
# The token is nowhere — and the username is gone from the page
|
|
# text too (the whole userinfo was stripped, not just the secret).
|
|
_assert_token_nowhere(page, app_url, TOKEN)
|
|
body_text = page.evaluate("() => document.body.innerText")
|
|
assert "myuser" not in body_text, "the username leaks into the page text"
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# 3. Edit: the editor's token field opens BLANK (never prefilled) and a
|
|
# blank save is a 200 no-change — the stored token is kept
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
def test_admin_editor_token_opens_blank_and_save_keeps_token(
|
|
page: Page, app_url: str, db_ready: None
|
|
) -> None:
|
|
page.set_default_timeout(30_000)
|
|
_seed_token_row(PRIVATE_URL, TOKEN) # before the page loads its list
|
|
_admin_git_sources_page(page, app_url)
|
|
row = page.locator("#git-sources-tbody tr", has_text=PRIVATE_URL)
|
|
expect(row).to_have_count(1, timeout=30_000)
|
|
|
|
# Open the per-row editor ("Ignore paths" — the row's settings).
|
|
row.locator(".git-source-ignore").click()
|
|
dialog = page.locator("#ignore-editor-dialog")
|
|
expect(dialog).to_be_visible(timeout=30_000)
|
|
expect(page.locator("#ignore-editor-source")).to_have_text(PRIVATE_URL)
|
|
|
|
# The token field: masked, and BLANK — a password must never be
|
|
# echoed back (there is no token field to prefill from — LOCKED A2).
|
|
tok = page.locator("#ignore-editor-token")
|
|
expect(tok).to_have_value("")
|
|
assert tok.get_attribute("type") == "password"
|
|
|
|
patch_bodies: list[dict[str, Any] | None] = []
|
|
patch_statuses: list[int] = []
|
|
|
|
def track_request(r: Any) -> None:
|
|
if r.method == "PATCH" and "/api/git-sources/" in r.url:
|
|
try:
|
|
patch_bodies.append(json.loads(r.post_data))
|
|
except (TypeError, ValueError):
|
|
patch_bodies.append(None)
|
|
|
|
def track_response(r: Any) -> None:
|
|
if r.request.method == "PATCH" and "/api/git-sources/" in r.url:
|
|
patch_statuses.append(r.status)
|
|
|
|
page.on("request", track_request)
|
|
page.on("response", track_response)
|
|
try:
|
|
# Save with the token field blank → 200 (no 4xx), the dialog
|
|
# closes, no inline error, the row is intact.
|
|
page.click("#ignore-editor-save")
|
|
expect(dialog).to_be_hidden(timeout=30_000)
|
|
expect(page.locator("#ignore-editor-error")).to_be_hidden()
|
|
expect(page.locator("#git-sources-tbody tr", has_text=PRIVATE_URL)).to_have_count(1)
|
|
finally:
|
|
page.remove_listener("request", track_request)
|
|
page.remove_listener("response", track_response)
|
|
|
|
# One PATCH, 200, and the body OMITS the token key (blank = the
|
|
# tri-state no-change — the key is not sent, let alone empty).
|
|
assert patch_statuses == [200], f"the blank-token save did not 200: {patch_statuses}"
|
|
assert len(patch_bodies) == 1 and patch_bodies[0] is not None
|
|
assert "token" not in patch_bodies[0], "a blank token must OMIT the PATCH key"
|
|
assert patch_bodies[0] == {"ignore_paths": []}
|
|
|
|
# The row is intact and the stored token is KEPT (the no-change
|
|
# tri-state, verified in the trusted store).
|
|
stored_url, stored_token = _stored_row()
|
|
assert stored_url == PRIVATE_URL
|
|
assert stored_token == TOKEN
|
|
_assert_token_nowhere(page, app_url, TOKEN)
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# 4. Cleanup: the confirmation-modal removal → 204, the list is empty
|
|
# again, and the token is gone from every surface
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
def test_admin_remove_private_repo_leaves_empty_list(
|
|
page: Page, app_url: str, db_ready: None
|
|
) -> None:
|
|
page.set_default_timeout(30_000)
|
|
_seed_token_row(PRIVATE_URL, TOKEN) # before the page loads its list
|
|
_admin_git_sources_page(page, app_url)
|
|
row = page.locator("#git-sources-tbody tr", has_text=PRIVATE_URL)
|
|
expect(row).to_have_count(1, timeout=30_000)
|
|
|
|
delete_statuses: list[int] = []
|
|
|
|
def track_response(r: Any) -> None:
|
|
if r.request.method == "DELETE" and "/api/git-sources/" in r.url:
|
|
delete_statuses.append(r.status)
|
|
|
|
page.on("response", track_response)
|
|
try:
|
|
row.locator(".git-source-remove").click()
|
|
dialog = page.locator("#remove-confirm-dialog")
|
|
expect(dialog).to_be_visible(timeout=30_000)
|
|
# The modal names the source BARE (the same sanitized value the
|
|
# row shows — a credential is never rendered here either).
|
|
expect(page.locator("#remove-confirm-source")).to_have_text(PRIVATE_URL)
|
|
page.click("#remove-confirm-remove")
|
|
expect(page.locator("#git-sources-tbody tr")).to_have_count(0, timeout=30_000)
|
|
expect(dialog).to_be_hidden()
|
|
finally:
|
|
page.remove_listener("response", track_response)
|
|
|
|
# Total removal: one DELETE, 204, and the list is empty again —
|
|
# the API agrees (and the row's token left with the row).
|
|
assert delete_statuses == [204], f"the removal did not 204: {delete_statuses}"
|
|
r = page.request.get(f"{app_url}/api/git-sources")
|
|
assert r.status == 200, r.text
|
|
assert r.json()["sources"] == []
|
|
assert TOKEN not in r.text()
|
|
_assert_token_nowhere(page, app_url, TOKEN)
|