**Phase 84 — final verification pass: all green, no defects found** - Verified implementation: `app/core/errors.py` (verbatim lift of sync masker), `app/api/sync.py` alias import, docs-push 502 `detail=sanitize_error(str(exc))`, all five `llm.py` error sites sanitized; new/extended test pins in place - Tests: `uv run pytest` → **1714 passed, 0 failed**; targeted pins (new unit ×2 + integration ×1, existing 502 pin) → 13 passed; sync/git-sources regression → 67 passed - Coverage: `uv run pytest --cov=app --cov-report=term-missing` → **99%** (`app/core/errors.py` 100%, `app/rag/llm.py` 100%) — >90% met - E2E isolation: `uv run pytest tests/e2e/test_smoke.py -v --no-cov` → **3 passed** - Lint/types: `uv run ruff check .` → clean; `uv run pyright` → **0 errors** - Criteria: 502 masks `*****@`/never token + row untouched ✅; LLM base-URL masked, credential-free strings byte-identical ✅; `_CREDS_RE` only in `app/core/errors.py` (working-tree grep) ✅; full gate green ✅; `git diff --stat` limited to the 4 app files + 2 modified test files + 3 phase task files (untracked: new module, new unit test, complete/ dir, reports, audit plan) ✅ - Commit/phase move left to the harness per instructions (task files already in `complete/`) - No deviations; nothing to fix - Next pending phase: **85_mobile_menu_gate_overlap**
305 lines
12 KiB
Python
305 lines
12 KiB
Python
"""Doc-drafts API — the draft lifecycle the edit screen runs on
|
|
(phase 59, task 02).
|
|
|
|
The "Save as doc" action (task 05) POSTs a completed answer here, the
|
|
edit screen (task 06) GETs/PUTs it by token, and the push endpoint
|
|
(task 04) commits + pushes it. A draft's long answer body lives on the
|
|
**server** — never in a URL: the row is keyed by an unguessable 128-bit
|
|
``uuid4`` ``token`` (the edit screen's URL credential,
|
|
``/doc-edit.html?draft=<token>`` — the share-token trust model,
|
|
phase 51).
|
|
|
|
The whole router sits behind :func:`app.core.auth.require_admin`
|
|
(router-wide ``dependencies`` — the :mod:`app.api.steering` pattern):
|
|
drafts are admin-only, so anonymous callers get 403 on every route.
|
|
|
|
Routes: ``POST /api/doc-drafts`` (create — 201, ``token = uuid4``,
|
|
``status = "draft"``), ``GET /api/doc-drafts/{token}`` (fetch by
|
|
token — 404 ``draft not found`` when unknown), ``PUT
|
|
/api/doc-drafts/{token}`` (partial update — absent fields unchanged,
|
|
``updated_at`` bumped; editing a ``pushed`` draft resets ``status``
|
|
back to ``draft`` — the stored sha no longer describes the current
|
|
body, so the next push re-commits; phase 59 D3 ASSUMPTION), ``POST
|
|
/api/doc-drafts/{token}/push`` (the single mutation the edit screen
|
|
triggers — commit + ``git push`` the draft's file to the
|
|
``BOR_DOCS_REPO`` ``BOR_DOCS_BRANCH`` via
|
|
:func:`app.core.docs_push.push_document`; success records
|
|
``status`` / ``branch`` / ``commit_sha`` on the row and returns
|
|
``DocDraftPushed``; 409 while unconfigured, 422 on a path that no
|
|
longer passes the guard-rails, 502 on git failure with the
|
|
SANITIZED git stderr in the detail (credential ``user:pass@``
|
|
userinfo masked by :func:`app.core.errors.sanitize_error` — phase
|
|
84, SEC-08) — the row untouched).
|
|
|
|
Every ``path`` (create, update **and** push) passes the shared
|
|
:func:`validate_draft_path` guard, so no draft can ever be created,
|
|
edited or pushed with a path that escapes the repo root.
|
|
"""
|
|
from __future__ import annotations
|
|
|
|
import uuid
|
|
from pathlib import Path
|
|
|
|
from fastapi import APIRouter, Depends, HTTPException
|
|
from sqlalchemy import select, text
|
|
from sqlalchemy.orm import Session
|
|
|
|
from app.config import Settings, get_settings
|
|
from app.core.auth import require_admin
|
|
from app.core.docs_push import DocsPushError, push_document
|
|
from app.core.errors import sanitize_error
|
|
from app.db import get_db
|
|
from app.models import DocDraft
|
|
from app.schemas import DocDraft as DocDraftOut
|
|
from app.schemas import (
|
|
DocDraftCreate,
|
|
DocDraftPushed,
|
|
DocDraftUpdate,
|
|
)
|
|
|
|
router = APIRouter(
|
|
prefix="/doc-drafts",
|
|
tags=["doc-drafts"],
|
|
dependencies=[Depends(require_admin)], # phase 59: drafts are admin-only
|
|
)
|
|
|
|
|
|
def validate_draft_path(raw: str) -> str:
|
|
"""Guard-rail an in-repo doc path (phase 59, task 02).
|
|
|
|
Shared by the create/update routes **and** the push endpoint (task
|
|
04): the file will later be written inside the ``BOR_DOCS_REPO``
|
|
checkout, so a path that escapes the repo root is a security hole,
|
|
not a typo. Rules (first violation wins, each 422 names its rule):
|
|
|
|
* non-empty after strip — a blank path names no file;
|
|
* not absolute — ``/etc/passwd`` would leave the checkout;
|
|
* no ``.``/``..`` components — ``../x.md`` and ``a/b/../c.md`` walk
|
|
out of the checkout (checked via ``Path(p).parts``, so the
|
|
traversal is rejected wherever it sits);
|
|
* carries a file suffix (``Path(p).suffix`` non-empty) — the
|
|
committed file must be a real file, e.g. ``docs/note.md``.
|
|
|
|
Returns the stripped path (the value that gets stored); raises
|
|
:class:`fastapi.HTTPException` (422) on the first violated rule.
|
|
"""
|
|
path = raw.strip()
|
|
if not path:
|
|
raise HTTPException(status_code=422, detail="path must not be empty")
|
|
if Path(path).is_absolute():
|
|
raise HTTPException(status_code=422, detail="path must not be an absolute path")
|
|
if any(part in (".", "..") for part in Path(path).parts):
|
|
raise HTTPException(
|
|
status_code=422,
|
|
detail="path must not contain '.' or '..' path components",
|
|
)
|
|
if not Path(path).suffix:
|
|
raise HTTPException(
|
|
status_code=422, detail="path must carry a file suffix (e.g. docs/note.md)"
|
|
)
|
|
return path
|
|
|
|
|
|
def _to_out(row: DocDraft) -> DocDraftOut:
|
|
"""The full-payload response shape (create/get/put)."""
|
|
return DocDraftOut(
|
|
token=row.token,
|
|
title=row.title,
|
|
path=row.path,
|
|
body=row.body,
|
|
status=row.status,
|
|
branch=row.branch,
|
|
commit_sha=row.commit_sha,
|
|
created_at=row.created_at,
|
|
updated_at=row.updated_at,
|
|
)
|
|
|
|
|
|
def _get_draft_or_404(db: Session, token: uuid.UUID) -> DocDraft:
|
|
"""One draft by its URL credential; 404 when the token is unknown."""
|
|
row = db.execute(select(DocDraft).where(DocDraft.token == token)).scalars().first()
|
|
if row is None:
|
|
raise HTTPException(status_code=404, detail="draft not found")
|
|
return row
|
|
|
|
|
|
@router.post("", response_model=DocDraftOut, status_code=201)
|
|
def create_draft(
|
|
payload: DocDraftCreate,
|
|
db: Session = Depends(get_db), # noqa: B008
|
|
) -> DocDraftOut:
|
|
"""Store one completed answer as a draft (201).
|
|
|
|
``title`` / ``body`` must be non-empty after strip (422 — the
|
|
``SteeringNoteIn`` pattern; pydantic's ``min_length=1`` alone would
|
|
let a whitespace-only value through). ``path`` passes
|
|
:func:`validate_draft_path` (422 naming the violated rule).
|
|
``token`` is a fresh ``uuid4`` — set on the PENDING row, so it ships
|
|
in the same INSERT (the ``share_token`` precedent, phase 51);
|
|
``status`` starts as ``draft``.
|
|
"""
|
|
title = payload.title.strip()
|
|
if not title:
|
|
raise HTTPException(status_code=422, detail="title must not be empty")
|
|
body = payload.body.strip()
|
|
if not body:
|
|
raise HTTPException(status_code=422, detail="body must not be empty")
|
|
path = validate_draft_path(payload.path)
|
|
|
|
row = DocDraft(
|
|
token=uuid.uuid4(), # the URL credential — set before INSERT
|
|
title=title,
|
|
path=path,
|
|
body=body,
|
|
)
|
|
db.add(row)
|
|
db.commit()
|
|
db.refresh(row)
|
|
return _to_out(row)
|
|
|
|
|
|
@router.get("/{token}", response_model=DocDraftOut)
|
|
def get_draft(
|
|
token: uuid.UUID,
|
|
db: Session = Depends(get_db), # noqa: B008
|
|
) -> DocDraftOut:
|
|
"""One draft by its URL credential (the edit screen's load); 404
|
|
``draft not found`` when the token is unknown (no enumeration — one
|
|
message for every unknown token)."""
|
|
return _to_out(_get_draft_or_404(db, token))
|
|
|
|
|
|
@router.put("/{token}", response_model=DocDraftOut)
|
|
def update_draft(
|
|
token: uuid.UUID,
|
|
payload: DocDraftUpdate,
|
|
db: Session = Depends(get_db), # noqa: B008
|
|
) -> DocDraftOut:
|
|
"""Partial edit of a draft (the edit screen's Save); 404 when the
|
|
token is unknown.
|
|
|
|
Each field is replaced only when supplied (absent keeps the row's
|
|
current value; present must be non-empty after strip — 422). A
|
|
supplied ``path`` re-runs :func:`validate_draft_path`; the same
|
|
guard-rails apply on update as on create. ``updated_at`` is bumped
|
|
on every PUT — via the model's ``onupdate=func.now()`` when a stored
|
|
value changes, via an explicit raw ``UPDATE`` when the PUT is a
|
|
no-op (empty body, or every supplied value identical — the ORM
|
|
flushes nothing, so the onupdate default never fires).
|
|
|
|
Editing a ``pushed`` draft resets ``status`` to ``draft`` (phase 59
|
|
D3 ASSUMPTION): the stored ``commit_sha`` no longer describes the
|
|
current body, so the next push re-commits. The last push's
|
|
``branch``/``commit_sha`` stay visible (the previous state) until
|
|
the next push overwrites them.
|
|
"""
|
|
row = _get_draft_or_404(db, token)
|
|
|
|
# Validate everything BEFORE assigning anything, so a 422 on one
|
|
# field can never leave a half-applied edit pending.
|
|
new_title = payload.title.strip() if payload.title is not None else None
|
|
if payload.title is not None and not new_title:
|
|
raise HTTPException(status_code=422, detail="title must not be empty")
|
|
new_path = validate_draft_path(payload.path) if payload.path is not None else None
|
|
new_body = payload.body.strip() if payload.body is not None else None
|
|
if payload.body is not None and not new_body:
|
|
raise HTTPException(status_code=422, detail="body must not be empty")
|
|
|
|
changed = False
|
|
if new_title is not None and row.title != new_title:
|
|
row.title = new_title
|
|
changed = True
|
|
if new_path is not None and row.path != new_path:
|
|
row.path = new_path
|
|
changed = True
|
|
if new_body is not None and row.body != new_body:
|
|
row.body = new_body
|
|
changed = True
|
|
if row.status == "pushed":
|
|
# D3 ASSUMPTION — see the docstring above.
|
|
row.status = "draft"
|
|
changed = True
|
|
|
|
if not changed:
|
|
# No-op PUT: the onupdate default does not fire for a flush that
|
|
# changed nothing, so bump updated_at explicitly (raw SQL — the
|
|
# ORM object is already in its final state).
|
|
db.execute(
|
|
text("UPDATE doc_drafts SET updated_at = now() WHERE id = :id"),
|
|
{"id": row.id},
|
|
)
|
|
db.commit()
|
|
db.refresh(row)
|
|
return _to_out(row)
|
|
|
|
|
|
@router.post("/{token}/push", response_model=DocDraftPushed)
|
|
def push_doc_draft(
|
|
token: uuid.UUID,
|
|
db: Session = Depends(get_db), # noqa: B008
|
|
settings: Settings = Depends(get_settings), # noqa: B008
|
|
) -> DocDraftPushed:
|
|
"""Commit + push the draft's file (phase 59, task 04).
|
|
|
|
The single mutation the edit screen (task 06) triggers: take the
|
|
draft's (title, path, body), commit + push through
|
|
:func:`app.core.docs_push.push_document`, and record the outcome
|
|
on the draft. Outcomes (checked in this order):
|
|
|
|
1. unknown token → 404 ``draft not found`` (same message as GET/
|
|
PUT — no enumeration);
|
|
2. ``settings.docs_configured`` false → 409 naming
|
|
``BOR_DOCS_REPO`` (D3: the feature is inert by default — the
|
|
optional-feature pattern of the git-sources env fallback);
|
|
3. the stored ``path`` re-runs :func:`validate_draft_path` → 422
|
|
on the first violated rule (a row must not be pushable into a
|
|
bad path, whatever wrote it);
|
|
4. :class:`DocsPushError` → 502 with the SANITIZED git stderr in
|
|
the detail (phase 84, SEC-08): ``str(exc)`` runs through
|
|
:func:`app.core.errors.sanitize_error`, which masks any
|
|
``user:pass@`` userinfo git's stderr may echo (a
|
|
``BOR_DOCS_REPO`` URL with embedded credentials) — the failing
|
|
repo and git's reason stay readable, and the
|
|
``GitSyncError`` → ``detail`` mapping from
|
|
:mod:`app.api.git_sources` is otherwise kept. Only a SUCCESS
|
|
mutates the row: the failed push leaves ``status`` / ``branch``
|
|
/ ``commit_sha`` exactly as found (no partial commit).
|
|
|
|
On success the row becomes ``status = "pushed"`` with the landed
|
|
``branch`` and ``commit_sha`` (must equal ``git rev-parse
|
|
<branch>`` in the repo) and a bumped ``updated_at`` (the column's
|
|
``onupdate`` fires — a successful push always rewrites at least
|
|
``commit_sha``, so a flush always happens); the response is
|
|
``DocDraftPushed(status="pushed", branch, commit_sha=sha)`` — the
|
|
edit screen's branch + sha feedback (D3: no PR, no URL — the
|
|
owner opens the PR themselves).
|
|
"""
|
|
row = _get_draft_or_404(db, token)
|
|
if not settings.docs_configured:
|
|
raise HTTPException(
|
|
status_code=409, detail="docs repo not configured (BOR_DOCS_REPO)"
|
|
)
|
|
path = validate_draft_path(row.path)
|
|
try:
|
|
branch, sha = push_document(
|
|
repo=settings.docs_repo,
|
|
base_branch=settings.docs_base_branch,
|
|
branch=settings.docs_branch,
|
|
work_dir=settings.docs_work_dir,
|
|
rel_path=path,
|
|
content=row.body,
|
|
commit_message=f"docs: {row.title}",
|
|
)
|
|
except DocsPushError as exc:
|
|
# Phase 84 (SEC-08): git's stderr may echo the remote URL with
|
|
# embedded credentials — the RESPONSE is where the secret would
|
|
# leak, so it is sanitized here (the DocsPushError itself keeps
|
|
# carrying the full stderr for logs/inspection).
|
|
raise HTTPException(status_code=502, detail=sanitize_error(str(exc))) from None
|
|
row.status = "pushed"
|
|
row.branch = branch
|
|
row.commit_sha = sha
|
|
db.commit()
|
|
db.refresh(row)
|
|
return DocDraftPushed(status="pushed", branch=branch, commit_sha=sha)
|