feat(forge): push webhook flags drift at the moment the repo moves (#2691)
CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 7s
CI & Build / TypeScript typecheck (push) Successful in 22s
CI & Build / integration (push) Successful in 22s
CI & Build / Python tests (push) Successful in 53s
CI & Build / Build & push image (push) Successful in 39s
CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 7s
CI & Build / TypeScript typecheck (push) Successful in 22s
CI & Build / integration (push) Successful in 22s
CI & Build / Python tests (push) Successful in 53s
CI & Build / Build & push image (push) Successful in 39s
Second adapter consumer. POST /api/webhooks/forge validates Gitea's
X-Gitea-Signature (HMAC-SHA256, constant-time; no secret configured =
the endpoint 404s out of existence), extracts changed/removed paths,
and flags matched snippets by writing verification.invalidated_by
{commit_sha, at, path, removed} — the existing attention vocabulary
extended, not a new flag: needs_attention includes it, both filter
dialects (Python + jsonpath SQL) include it in 'attention' and exclude
it from 'ok', and recording ANY fresh verdict clears it by construction
because compose_verification builds a new dict. Unverified snippets are
skipped (already in their own bucket); replayed deliveries at the same
head commit are no-ops; processing failures return 200 with a WARNING +
AppLog canary so the forge never marks deliveries failed and operators
never disable the hook over a transient (#2663's lesson).
Matching goes through repo BINDINGS: recorded location repos are
free-form names ('Scribe') that cannot address a forge, so a snippet
reaches its forge repo through its project's binding — which also fixes
step 5's pull-time resolution for every real record via the same
fallback. O(bindings + snippets-in-project + changed files).
Settings: webhook secret beside the forge config (masked, sentinel-
skipped, Docker-secret env channel, endpoint documented in the UI).
Tests: signature gate, payload parsing, path semantics, both filter
dialects extended in the drift-check guard file, and real-Postgres
end-to-end (flag lands, attention lists it, replay quiet, re-verify
clears, unbound repo untouched).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -422,7 +422,7 @@ const baseUrlSaved = ref(false);
|
|||||||
|
|
||||||
// Git forge integration (admin only, #2689). The token round-trips masked;
|
// Git forge integration (admin only, #2689). The token round-trips masked;
|
||||||
// the server treats the mask as "unchanged".
|
// the server treats the mask as "unchanged".
|
||||||
const forge = ref({ kind: "", base_url: "", token: "" });
|
const forge = ref({ kind: "", base_url: "", token: "", webhook_secret: "" });
|
||||||
const forgeKinds = ref<string[]>(["gitea"]);
|
const forgeKinds = ref<string[]>(["gitea"]);
|
||||||
const forgeConfigured = ref(false);
|
const forgeConfigured = ref(false);
|
||||||
const savingForge = ref(false);
|
const savingForge = ref(false);
|
||||||
@@ -586,10 +586,13 @@ onMounted(async () => {
|
|||||||
|
|
||||||
async function loadForgeSettings() {
|
async function loadForgeSettings() {
|
||||||
const cfg = await apiGet<{
|
const cfg = await apiGet<{
|
||||||
kind: string; base_url: string; token: string;
|
kind: string; base_url: string; token: string; webhook_secret: string;
|
||||||
configured: boolean; kinds: string[];
|
configured: boolean; kinds: string[];
|
||||||
}>("/api/admin/forge");
|
}>("/api/admin/forge");
|
||||||
forge.value = { kind: cfg.kind, base_url: cfg.base_url, token: cfg.token };
|
forge.value = {
|
||||||
|
kind: cfg.kind, base_url: cfg.base_url, token: cfg.token,
|
||||||
|
webhook_secret: cfg.webhook_secret,
|
||||||
|
};
|
||||||
forgeConfigured.value = cfg.configured;
|
forgeConfigured.value = cfg.configured;
|
||||||
if (cfg.kinds?.length) forgeKinds.value = cfg.kinds;
|
if (cfg.kinds?.length) forgeKinds.value = cfg.kinds;
|
||||||
}
|
}
|
||||||
@@ -2177,6 +2180,15 @@ function formatUserDate(iso: string): string {
|
|||||||
<label for="forge-token">API Token (read scope)</label>
|
<label for="forge-token">API Token (read scope)</label>
|
||||||
<input id="forge-token" v-model="forge.token" type="password" class="input" />
|
<input id="forge-token" v-model="forge.token" type="password" class="input" />
|
||||||
</div>
|
</div>
|
||||||
|
<div class="field">
|
||||||
|
<label for="forge-webhook-secret">Webhook Secret</label>
|
||||||
|
<input id="forge-webhook-secret" v-model="forge.webhook_secret" type="password" class="input" />
|
||||||
|
<p class="field-hint">
|
||||||
|
Optional: create a push webhook on the forge pointing at
|
||||||
|
<code>/api/webhooks/forge</code> with this secret, and snippets
|
||||||
|
whose recorded files change get flagged for re-verification.
|
||||||
|
</p>
|
||||||
|
</div>
|
||||||
</div>
|
</div>
|
||||||
<div class="actions" style="margin-bottom: 1.25rem;">
|
<div class="actions" style="margin-bottom: 1.25rem;">
|
||||||
<button class="btn-primary" @click="saveForge" :disabled="savingForge">
|
<button class="btn-primary" @click="saveForge" :disabled="savingForge">
|
||||||
|
|||||||
@@ -31,6 +31,7 @@ from scribe.routes.trash import trash_bp
|
|||||||
from scribe.routes.dashboard import dashboard_bp
|
from scribe.routes.dashboard import dashboard_bp
|
||||||
from scribe.routes.systems import systems_bp
|
from scribe.routes.systems import systems_bp
|
||||||
from scribe.routes.snippets import snippets_bp
|
from scribe.routes.snippets import snippets_bp
|
||||||
|
from scribe.routes.webhooks import webhooks_bp
|
||||||
from scribe.mcp import mount_mcp
|
from scribe.mcp import mount_mcp
|
||||||
|
|
||||||
STATIC_DIR = Path(__file__).parent / "static"
|
STATIC_DIR = Path(__file__).parent / "static"
|
||||||
@@ -95,6 +96,7 @@ def create_app() -> Quart:
|
|||||||
app.register_blueprint(dashboard_bp)
|
app.register_blueprint(dashboard_bp)
|
||||||
app.register_blueprint(systems_bp)
|
app.register_blueprint(systems_bp)
|
||||||
app.register_blueprint(snippets_bp)
|
app.register_blueprint(snippets_bp)
|
||||||
|
app.register_blueprint(webhooks_bp)
|
||||||
|
|
||||||
@app.before_request
|
@app.before_request
|
||||||
async def before_request():
|
async def before_request():
|
||||||
|
|||||||
@@ -68,6 +68,9 @@ class Config:
|
|||||||
FORGE_KIND: str = os.environ.get("FORGE_KIND", "")
|
FORGE_KIND: str = os.environ.get("FORGE_KIND", "")
|
||||||
FORGE_BASE_URL: str = os.environ.get("FORGE_BASE_URL", "").rstrip("/")
|
FORGE_BASE_URL: str = os.environ.get("FORGE_BASE_URL", "").rstrip("/")
|
||||||
FORGE_TOKEN: str = _read_secret("FORGE_TOKEN", "FORGE_TOKEN_FILE", "")
|
FORGE_TOKEN: str = _read_secret("FORGE_TOKEN", "FORGE_TOKEN_FILE", "")
|
||||||
|
FORGE_WEBHOOK_SECRET: str = _read_secret(
|
||||||
|
"FORGE_WEBHOOK_SECRET", "FORGE_WEBHOOK_SECRET_FILE", ""
|
||||||
|
)
|
||||||
|
|
||||||
@classmethod
|
@classmethod
|
||||||
def oidc_enabled(cls) -> bool:
|
def oidc_enabled(cls) -> bool:
|
||||||
|
|||||||
@@ -172,13 +172,21 @@ _TOKEN_MASK = "********"
|
|||||||
@admin_bp.route("/forge", methods=["GET"])
|
@admin_bp.route("/forge", methods=["GET"])
|
||||||
@admin_required
|
@admin_required
|
||||||
async def get_forge_settings():
|
async def get_forge_settings():
|
||||||
|
from scribe.config import Config
|
||||||
|
from scribe.routes.webhooks import FORGE_WEBHOOK_SECRET_KEY
|
||||||
|
|
||||||
cfg = await forge_config()
|
cfg = await forge_config()
|
||||||
|
webhook_secret = (
|
||||||
|
await get_admin_setting(FORGE_WEBHOOK_SECRET_KEY, "")
|
||||||
|
or Config.FORGE_WEBHOOK_SECRET
|
||||||
|
)
|
||||||
return jsonify({
|
return jsonify({
|
||||||
"kind": cfg["kind"],
|
"kind": cfg["kind"],
|
||||||
"base_url": cfg["base_url"],
|
"base_url": cfg["base_url"],
|
||||||
# The token itself never leaves the server — the smtp_password
|
# Secrets never leave the server — the smtp_password convention:
|
||||||
# convention: masked when set, empty when not.
|
# masked when set, empty when not.
|
||||||
"token": _TOKEN_MASK if cfg["token"] else "",
|
"token": _TOKEN_MASK if cfg["token"] else "",
|
||||||
|
"webhook_secret": _TOKEN_MASK if webhook_secret else "",
|
||||||
"configured": bool(await get_forge()),
|
"configured": bool(await get_forge()),
|
||||||
"kinds": list(FORGE_KINDS),
|
"kinds": list(FORGE_KINDS),
|
||||||
})
|
})
|
||||||
@@ -204,6 +212,11 @@ async def update_forge_settings():
|
|||||||
# showed it, and storing the mask would silently break the integration.
|
# showed it, and storing the mask would silently break the integration.
|
||||||
if token is not None and token != _TOKEN_MASK:
|
if token is not None and token != _TOKEN_MASK:
|
||||||
await set_admin_setting(FORGE_TOKEN_KEY, str(token))
|
await set_admin_setting(FORGE_TOKEN_KEY, str(token))
|
||||||
|
from scribe.routes.webhooks import FORGE_WEBHOOK_SECRET_KEY
|
||||||
|
|
||||||
|
webhook_secret = data.get("webhook_secret")
|
||||||
|
if webhook_secret is not None and webhook_secret != _TOKEN_MASK:
|
||||||
|
await set_admin_setting(FORGE_WEBHOOK_SECRET_KEY, str(webhook_secret))
|
||||||
# The token is deliberately absent from the audit detail.
|
# The token is deliberately absent from the audit detail.
|
||||||
await log_audit(
|
await log_audit(
|
||||||
"forge_config", user_id=uid, username=g.user.username,
|
"forge_config", user_id=uid, username=g.user.username,
|
||||||
|
|||||||
@@ -20,7 +20,7 @@ settings_bp = Blueprint("settings", __name__, url_prefix="/api/settings")
|
|||||||
# read and skip the mask on write; this generic KV surface has to apply the
|
# read and skip the mask on write; this generic KV surface has to apply the
|
||||||
# same treatment, or it silently un-masks what those endpoints masked — the
|
# same treatment, or it silently un-masks what those endpoints masked — the
|
||||||
# rows live on the admin's own user_id, so the plain GET returned them raw.
|
# rows live on the admin's own user_id, so the plain GET returned them raw.
|
||||||
_SECRET_KEYS = frozenset({"smtp_password", "forge_token"})
|
_SECRET_KEYS = frozenset({"smtp_password", "forge_token", "forge_webhook_secret"})
|
||||||
_SECRET_MASK = "********"
|
_SECRET_MASK = "********"
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -0,0 +1,107 @@
|
|||||||
|
"""Forge push webhook — drift flagging at the moment the repo moves (#2691).
|
||||||
|
|
||||||
|
The forge POSTs here on every push; changed paths are matched against recorded
|
||||||
|
snippet locations (through repo bindings) and matched verdicts get an
|
||||||
|
``invalidated_by`` marker that surfaces in the ``verification="attention"``
|
||||||
|
listing. This is what makes verification scale past tens of records: sessions
|
||||||
|
recheck what pushes flagged instead of sweeping everything.
|
||||||
|
|
||||||
|
Registering the webhook on the forge is per-instance setup (Settings → Config
|
||||||
|
→ Git Forge shows the endpoint and holds the secret) — the server never
|
||||||
|
self-registers on the forge.
|
||||||
|
|
||||||
|
Contract with the forge's delivery loop:
|
||||||
|
- No secret configured → 404: the endpoint doesn't exist until an operator
|
||||||
|
creates it. Bad signature → 401: that's a caller problem worth signaling.
|
||||||
|
- A PROCESSING failure returns 200 with ``{"ok": false}`` and drops a
|
||||||
|
WARNING + AppLog row (the #2663 canary pattern): repeated 5xx responses
|
||||||
|
make forges mark deliveries failed and operators disable the hook, which
|
||||||
|
would silently turn the feature off — the exact failure mode this
|
||||||
|
milestone exists to prevent.
|
||||||
|
"""
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import hashlib
|
||||||
|
import hmac
|
||||||
|
import logging
|
||||||
|
import traceback
|
||||||
|
|
||||||
|
from quart import Blueprint, jsonify, request
|
||||||
|
|
||||||
|
from scribe.config import Config
|
||||||
|
from scribe.services.repo_bindings import normalize_repo_key
|
||||||
|
from scribe.services.settings import get_admin_setting
|
||||||
|
from scribe.services.snippets import invalidate_for_push
|
||||||
|
|
||||||
|
logger = logging.getLogger(__name__)
|
||||||
|
|
||||||
|
webhooks_bp = Blueprint("webhooks", __name__, url_prefix="/api/webhooks")
|
||||||
|
|
||||||
|
FORGE_WEBHOOK_SECRET_KEY = "forge_webhook_secret"
|
||||||
|
|
||||||
|
|
||||||
|
def signature_ok(secret: str, body: bytes, signature: str) -> bool:
|
||||||
|
"""Validate Gitea's push signature: X-Gitea-Signature is the hex HMAC-SHA256
|
||||||
|
of the raw body under the webhook secret. Constant-time compare."""
|
||||||
|
if not secret or not signature:
|
||||||
|
return False
|
||||||
|
expected = hmac.new(secret.encode("utf-8"), body, hashlib.sha256).hexdigest()
|
||||||
|
return hmac.compare_digest(expected, signature.strip().lower())
|
||||||
|
|
||||||
|
|
||||||
|
def push_facts(payload: dict) -> tuple[str, list[str], list[str], str]:
|
||||||
|
"""(repo identity, changed paths, removed paths, head commit) from a Gitea
|
||||||
|
push payload. Tolerant: absent fields read as empty, never raise."""
|
||||||
|
repo = payload.get("repository") or {}
|
||||||
|
raw_repo = repo.get("clone_url") or repo.get("html_url") or repo.get("full_name") or ""
|
||||||
|
changed: list[str] = []
|
||||||
|
removed: list[str] = []
|
||||||
|
for commit in payload.get("commits") or []:
|
||||||
|
changed.extend(commit.get("added") or [])
|
||||||
|
changed.extend(commit.get("modified") or [])
|
||||||
|
removed.extend(commit.get("removed") or [])
|
||||||
|
# De-dup while keeping order stable for logs.
|
||||||
|
changed = list(dict.fromkeys(changed))
|
||||||
|
removed = list(dict.fromkeys(removed))
|
||||||
|
return raw_repo, changed, removed, str(payload.get("after") or "")
|
||||||
|
|
||||||
|
|
||||||
|
@webhooks_bp.route("/forge", methods=["POST"])
|
||||||
|
async def forge_push():
|
||||||
|
secret = await get_admin_setting(FORGE_WEBHOOK_SECRET_KEY, "") or Config.FORGE_WEBHOOK_SECRET
|
||||||
|
if not secret:
|
||||||
|
# Not "forbidden" — the endpoint is not a thing on this instance.
|
||||||
|
return jsonify({"error": "Not found"}), 404
|
||||||
|
|
||||||
|
body = await request.get_data()
|
||||||
|
if not signature_ok(secret, body, request.headers.get("X-Gitea-Signature", "")):
|
||||||
|
return jsonify({"error": "Invalid signature"}), 401
|
||||||
|
|
||||||
|
try:
|
||||||
|
payload = await request.get_json(force=True) or {}
|
||||||
|
raw_repo, changed, removed, head = push_facts(payload)
|
||||||
|
repo_key = normalize_repo_key(raw_repo)
|
||||||
|
if not repo_key:
|
||||||
|
return jsonify({"ok": True, "flagged": 0, "reason": "no repository in payload"})
|
||||||
|
flagged = await invalidate_for_push(repo_key, changed, removed, head)
|
||||||
|
if flagged:
|
||||||
|
logger.info(
|
||||||
|
"forge push %s flagged %d snippet(s) for recheck", head[:12], flagged
|
||||||
|
)
|
||||||
|
return jsonify({"ok": True, "flagged": flagged})
|
||||||
|
except Exception:
|
||||||
|
logger.warning("forge webhook processing failed", exc_info=True)
|
||||||
|
try:
|
||||||
|
from scribe.services.logging import log_error
|
||||||
|
|
||||||
|
await log_error(
|
||||||
|
endpoint="webhooks/forge",
|
||||||
|
error_type="forge_webhook_failed",
|
||||||
|
error_message="push received but drift flagging failed — "
|
||||||
|
"snippets touched by this push were not marked for recheck",
|
||||||
|
traceback=traceback.format_exc(),
|
||||||
|
)
|
||||||
|
except Exception:
|
||||||
|
logger.debug("forge webhook canary write failed", exc_info=True)
|
||||||
|
# 200 on purpose — see the module docstring's delivery contract.
|
||||||
|
return jsonify({"ok": False})
|
||||||
@@ -142,20 +142,25 @@ def verification_matches(data: dict | None, value: str) -> bool:
|
|||||||
if not status:
|
if not status:
|
||||||
return want == "unverified"
|
return want == "unverified"
|
||||||
expired = verdict.get("code_sha") != (data or {}).get("code_sha")
|
expired = verdict.get("code_sha") != (data or {}).get("code_sha")
|
||||||
|
# A push touched the recorded location since the verdict (#2691): the repo
|
||||||
|
# moved under it, so it needs a look even though it hasn't failed.
|
||||||
|
invalidated = bool(verdict.get("invalidated_by"))
|
||||||
if want == "unverified":
|
if want == "unverified":
|
||||||
return False
|
return False
|
||||||
if want == "drifted":
|
if want == "drifted":
|
||||||
return status != "ok"
|
return status != "ok"
|
||||||
if want == "attention":
|
if want == "attention":
|
||||||
return status != "ok" or expired
|
return status != "ok" or expired or invalidated
|
||||||
if want == "ok":
|
if want == "ok":
|
||||||
return status == "ok" and not expired
|
return status == "ok" and not expired and not invalidated
|
||||||
return status == want
|
return status == want
|
||||||
|
|
||||||
|
|
||||||
_VERIFY_DRIFTED_JSONPATH = '$.verification ? (@.status != "ok")'
|
_VERIFY_DRIFTED_JSONPATH = '$.verification ? (@.status != "ok")'
|
||||||
_VERIFY_EXPIRED_JSONPATH = "$ ? (@.verification.code_sha != @.code_sha)"
|
_VERIFY_EXPIRED_JSONPATH = "$ ? (@.verification.code_sha != @.code_sha)"
|
||||||
_VERIFY_ANY_JSONPATH = "$.verification"
|
_VERIFY_ANY_JSONPATH = "$.verification"
|
||||||
|
# A push touched the recorded location since the verdict (#2691).
|
||||||
|
_VERIFY_INVALIDATED_JSONPATH = "$.verification.invalidated_by"
|
||||||
|
|
||||||
|
|
||||||
def _verification_clause(value: str):
|
def _verification_clause(value: str):
|
||||||
@@ -171,19 +176,23 @@ def _verification_clause(value: str):
|
|||||||
if want == "drifted":
|
if want == "drifted":
|
||||||
return Note.data.path_exists(_VERIFY_DRIFTED_JSONPATH)
|
return Note.data.path_exists(_VERIFY_DRIFTED_JSONPATH)
|
||||||
if want == "attention":
|
if want == "attention":
|
||||||
# Everything worth looking at: a failing verdict, OR an expired one.
|
# Everything worth looking at: a failing verdict, an expired one, OR
|
||||||
|
# one whose recorded location a push has since touched (#2691).
|
||||||
return or_(
|
return or_(
|
||||||
Note.data.path_exists(_VERIFY_DRIFTED_JSONPATH),
|
Note.data.path_exists(_VERIFY_DRIFTED_JSONPATH),
|
||||||
and_(has_verdict, Note.data.path_exists(_VERIFY_EXPIRED_JSONPATH)),
|
and_(has_verdict, Note.data.path_exists(_VERIFY_EXPIRED_JSONPATH)),
|
||||||
|
Note.data.path_exists(_VERIFY_INVALIDATED_JSONPATH),
|
||||||
)
|
)
|
||||||
if want == "ok":
|
if want == "ok":
|
||||||
# A clean bill of health that still describes the current code. The
|
# A clean bill of health that still describes the current code. The
|
||||||
# `~expired` half matters: without it this would quietly include records
|
# `~expired` half matters: without it this would quietly include records
|
||||||
# whose blessing has lapsed, which is the exact failure the feature is
|
# whose blessing has lapsed, which is the exact failure the feature is
|
||||||
# meant to catch.
|
# meant to catch. Same for push-invalidation — "ok" must mean the repo
|
||||||
|
# hasn't moved under the verdict either.
|
||||||
return and_(
|
return and_(
|
||||||
Note.data.path_exists('$.verification ? (@.status == "ok")'),
|
Note.data.path_exists('$.verification ? (@.status == "ok")'),
|
||||||
~Note.data.path_exists(_VERIFY_EXPIRED_JSONPATH),
|
~Note.data.path_exists(_VERIFY_EXPIRED_JSONPATH),
|
||||||
|
~Note.data.path_exists(_VERIFY_INVALIDATED_JSONPATH),
|
||||||
)
|
)
|
||||||
# A specific status: 'missing' | 'moved' | 'changed'.
|
# A specific status: 'missing' | 'moved' | 'changed'.
|
||||||
return Note.data.path_exists(
|
return Note.data.path_exists(
|
||||||
@@ -229,6 +238,10 @@ def _note_to_item(note: Note) -> dict:
|
|||||||
"detail": verdict.get("detail"),
|
"detail": verdict.get("detail"),
|
||||||
"path": verdict.get("path"),
|
"path": verdict.get("path"),
|
||||||
}
|
}
|
||||||
|
# Present only when a push has touched the recorded location since the
|
||||||
|
# verdict (#2691) — the "recheck me" marker, cleared by re-verifying.
|
||||||
|
if verdict.get("invalidated_by"):
|
||||||
|
item["verification"]["invalidated_by"] = verdict["invalidated_by"]
|
||||||
|
|
||||||
# Task fields — override note_type and add status/priority/due_date
|
# Task fields — override note_type and add status/priority/due_date
|
||||||
if note.is_task:
|
if note.is_task:
|
||||||
|
|||||||
@@ -100,6 +100,41 @@ async def list_bindings(user_id: int) -> list[RepoBinding]:
|
|||||||
return list(rows.scalars().all())
|
return list(rows.scalars().all())
|
||||||
|
|
||||||
|
|
||||||
|
async def keys_for_project(user_id: int, project_id: int) -> list[str]:
|
||||||
|
"""Every repo key bound to a project — the snippet→forge join (#2691).
|
||||||
|
|
||||||
|
Recorded snippet locations carry free-form repo names ("Scribe"), which
|
||||||
|
can't address a forge API. The project's binding is the identity that can:
|
||||||
|
a snippet reaches its forge repo through the project it belongs to.
|
||||||
|
"""
|
||||||
|
async with async_session() as session:
|
||||||
|
rows = await session.execute(
|
||||||
|
select(RepoBinding.repo_key).where(
|
||||||
|
RepoBinding.user_id == user_id,
|
||||||
|
RepoBinding.project_id == project_id,
|
||||||
|
)
|
||||||
|
)
|
||||||
|
return [k for (k,) in rows.all()]
|
||||||
|
|
||||||
|
|
||||||
|
async def bindings_for_key(raw_repo: str) -> list[RepoBinding]:
|
||||||
|
"""All bindings (ANY user) for a repo key — the webhook's entry point.
|
||||||
|
|
||||||
|
A push webhook carries no Scribe caller, only the repository it happened
|
||||||
|
to; the flag it writes is about each record's truth, so every user who
|
||||||
|
bound the repo gets their project's snippets considered — each write still
|
||||||
|
lands as that record's owner.
|
||||||
|
"""
|
||||||
|
key = normalize_repo_key(raw_repo)
|
||||||
|
if not key:
|
||||||
|
return []
|
||||||
|
async with async_session() as session:
|
||||||
|
rows = await session.execute(
|
||||||
|
select(RepoBinding).where(RepoBinding.repo_key == key)
|
||||||
|
)
|
||||||
|
return list(rows.scalars().all())
|
||||||
|
|
||||||
|
|
||||||
async def delete_binding(user_id: int, raw_repo: str) -> bool:
|
async def delete_binding(user_id: int, raw_repo: str) -> bool:
|
||||||
"""Remove a repo's binding. Returns True if a row was deleted."""
|
"""Remove a repo's binding. Returns True if a row was deleted."""
|
||||||
key = normalize_repo_key(raw_repo)
|
key = normalize_repo_key(raw_repo)
|
||||||
|
|||||||
@@ -468,10 +468,19 @@ def verification_view(note, fields: dict) -> dict:
|
|||||||
"detail": stored.get("detail"),
|
"detail": stored.get("detail"),
|
||||||
"path": stored.get("path"),
|
"path": stored.get("path"),
|
||||||
"commit_sha": stored.get("commit_sha"),
|
"commit_sha": stored.get("commit_sha"),
|
||||||
|
# A push touched the recorded location since this verdict (#2691) —
|
||||||
|
# the repo moved under it. Cleared by the next verdict, which builds
|
||||||
|
# a fresh dict.
|
||||||
|
"invalidated_by": stored.get("invalidated_by"),
|
||||||
# What the operator actually wants to know: is there something to fix?
|
# What the operator actually wants to know: is there something to fix?
|
||||||
# An expired verdict counts as "needs looking at" even if it said ok,
|
# An expired verdict counts as "needs looking at" even if it said ok,
|
||||||
# since the code it blessed is not the code that's there now.
|
# since the code it blessed is not the code that's there now — and so
|
||||||
"needs_attention": (not current) or stored["status"] in VERIFY_DRIFTED,
|
# does a push-invalidated one, for the same reason from the repo side.
|
||||||
|
"needs_attention": (
|
||||||
|
(not current)
|
||||||
|
or stored["status"] in VERIFY_DRIFTED
|
||||||
|
or bool(stored.get("invalidated_by"))
|
||||||
|
),
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
@@ -1020,7 +1029,18 @@ async def attach_live_body(note, data: dict) -> None:
|
|||||||
data["body_source"] = "cache"
|
data["body_source"] = "cache"
|
||||||
data["body_freshness"] = "no-recorded-location"
|
data["body_freshness"] = "no-recorded-location"
|
||||||
return
|
return
|
||||||
|
# Recorded location repos are free-form names ("Scribe"), which can't
|
||||||
|
# address a forge API — the project's repo BINDING is the identity that
|
||||||
|
# can (#2691). Try the location string first (it may be a real remote),
|
||||||
|
# then fall back to the bindings of the snippet's project.
|
||||||
repo = forge.resolve_repo(loc["repo"])
|
repo = forge.resolve_repo(loc["repo"])
|
||||||
|
if repo is None and getattr(note, "project_id", None):
|
||||||
|
from scribe.services.repo_bindings import keys_for_project
|
||||||
|
|
||||||
|
for key in await keys_for_project(note.user_id, note.project_id):
|
||||||
|
repo = forge.resolve_repo(key)
|
||||||
|
if repo is not None:
|
||||||
|
break
|
||||||
if repo is None:
|
if repo is None:
|
||||||
data["body_source"] = "cache"
|
data["body_source"] = "cache"
|
||||||
data["body_freshness"] = "repo-not-on-this-forge"
|
data["body_freshness"] = "repo-not-on-this-forge"
|
||||||
@@ -1070,6 +1090,106 @@ async def attach_live_body(note, data: dict) -> None:
|
|||||||
data["body_freshness"] = "diverged"
|
data["body_freshness"] = "diverged"
|
||||||
|
|
||||||
|
|
||||||
|
# --- push-time drift flagging (#2691) ----------------------------------------
|
||||||
|
|
||||||
|
def _path_touches(recorded: str, changed: str) -> bool:
|
||||||
|
"""The location-path semantics, applied to a pushed file: the recorded path
|
||||||
|
is the changed file itself, or a directory above it."""
|
||||||
|
recorded = (recorded or "").strip("/")
|
||||||
|
changed = (changed or "").strip("/")
|
||||||
|
if not recorded or not changed:
|
||||||
|
return False
|
||||||
|
return changed == recorded or changed.startswith(recorded + "/")
|
||||||
|
|
||||||
|
|
||||||
|
async def invalidate_for_push(
|
||||||
|
repo_key: str,
|
||||||
|
changed: list[str],
|
||||||
|
removed: list[str],
|
||||||
|
commit_sha: str,
|
||||||
|
) -> int:
|
||||||
|
"""Flag snippets whose recorded location a push just touched (#2691).
|
||||||
|
|
||||||
|
Writes ``verification.invalidated_by = {commit_sha, at, path, removed}``
|
||||||
|
onto matched snippets that CARRY a verdict — the flag means "the repo
|
||||||
|
moved under this verdict, recheck it", and it clears itself the moment a
|
||||||
|
fresh verdict is recorded because compose_verification builds a new dict.
|
||||||
|
Unverified snippets are skipped: they are already in the unverified
|
||||||
|
bucket, and stacking a second unchecked-flavored flag on them adds noise,
|
||||||
|
not information.
|
||||||
|
|
||||||
|
Matching goes through repo BINDINGS (any user's — a webhook has no
|
||||||
|
caller): each binding names a project, and that project's snippets are
|
||||||
|
path-matched against the pushed files. O(bindings + snippets-in-project +
|
||||||
|
changed files); nothing else is scanned. Returns how many records were
|
||||||
|
newly flagged (an already-flagged record at the same commit is skipped,
|
||||||
|
so replayed deliveries don't churn).
|
||||||
|
"""
|
||||||
|
from scribe.services.repo_bindings import bindings_for_key
|
||||||
|
|
||||||
|
bindings = await bindings_for_key(repo_key)
|
||||||
|
if not bindings or not (changed or removed):
|
||||||
|
return 0
|
||||||
|
touched = [(p, False) for p in changed] + [(p, True) for p in removed]
|
||||||
|
|
||||||
|
flagged = 0
|
||||||
|
for binding in bindings:
|
||||||
|
async with async_session() as session:
|
||||||
|
rows = await session.execute(
|
||||||
|
select(Note).where(
|
||||||
|
Note.user_id == binding.user_id,
|
||||||
|
Note.project_id == binding.project_id,
|
||||||
|
Note.note_type == SNIPPET_NOTE_TYPE,
|
||||||
|
Note.deleted_at.is_(None),
|
||||||
|
Note.data.path_exists("$.verification"),
|
||||||
|
)
|
||||||
|
)
|
||||||
|
notes = list(rows.scalars().all())
|
||||||
|
for note in notes:
|
||||||
|
fields = snippet_fields(note)
|
||||||
|
verdict = fields.get("verification") or {}
|
||||||
|
if not verdict.get("status"):
|
||||||
|
continue
|
||||||
|
hit = next(
|
||||||
|
(
|
||||||
|
(path, was_removed)
|
||||||
|
for loc in (fields.get("locations") or [])
|
||||||
|
for path, was_removed in touched
|
||||||
|
if _path_touches(loc.get("path") or "", path)
|
||||||
|
),
|
||||||
|
None,
|
||||||
|
)
|
||||||
|
if hit is None:
|
||||||
|
continue
|
||||||
|
existing = verdict.get("invalidated_by") or {}
|
||||||
|
if existing.get("commit_sha") == commit_sha:
|
||||||
|
continue # replayed delivery — already says exactly this
|
||||||
|
verdict = dict(verdict)
|
||||||
|
verdict["invalidated_by"] = {
|
||||||
|
"commit_sha": commit_sha,
|
||||||
|
"at": datetime.now(timezone.utc).isoformat(),
|
||||||
|
"path": hit[0],
|
||||||
|
# A removed file is the strongest signal — the recorded
|
||||||
|
# location may simply be gone. Surfaced so the attention row
|
||||||
|
# says which kind of look it needs.
|
||||||
|
"removed": hit[1],
|
||||||
|
}
|
||||||
|
data = compose_data(
|
||||||
|
name=fields.get("name", ""),
|
||||||
|
when_to_use=fields.get("when_to_use", ""),
|
||||||
|
signature=fields.get("signature", ""),
|
||||||
|
language=fields.get("language", ""),
|
||||||
|
code=fields.get("code", ""),
|
||||||
|
locations=fields.get("locations") or [],
|
||||||
|
merged_from=fields.get("merged_from") or [],
|
||||||
|
verification=verdict,
|
||||||
|
provenance=fields.get("provenance"),
|
||||||
|
)
|
||||||
|
await notes_svc.update_note(note.user_id, note.id, data=data)
|
||||||
|
flagged += 1
|
||||||
|
return flagged
|
||||||
|
|
||||||
|
|
||||||
async def delete_snippet(user_id: int, snippet_id: int) -> bool:
|
async def delete_snippet(user_id: int, snippet_id: int) -> bool:
|
||||||
"""Retire a snippet to the trash (recoverable). Returns False if the id isn't
|
"""Retire a snippet to the trash (recoverable). Returns False if the id isn't
|
||||||
a snippet this user may WRITE.
|
a snippet this user may WRITE.
|
||||||
|
|||||||
@@ -0,0 +1,210 @@
|
|||||||
|
"""Forge push webhook (#2691) — signature gate, payload parsing, and the
|
||||||
|
end-to-end drift flag on real Postgres.
|
||||||
|
|
||||||
|
The webhook is the seam that makes verification scale: a push names exactly
|
||||||
|
which files moved, so only the records that point at them get flagged. The
|
||||||
|
properties pinned here:
|
||||||
|
|
||||||
|
- No secret configured → the endpoint does not exist (404); a bad
|
||||||
|
signature → 401. Both BEFORE any payload parsing.
|
||||||
|
- Matching is O(bindings + snippets-in-project + changed files) and goes
|
||||||
|
through repo bindings — recorded location repos are free-form names and
|
||||||
|
cannot address a forge.
|
||||||
|
- A replayed delivery (same head commit) flags nothing new; re-verifying
|
||||||
|
clears the flag by construction.
|
||||||
|
"""
|
||||||
|
import hashlib
|
||||||
|
import hmac
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
import pytest_asyncio
|
||||||
|
|
||||||
|
from scribe.routes.webhooks import push_facts, signature_ok
|
||||||
|
from scribe.services.snippets import _path_touches
|
||||||
|
|
||||||
|
SECRET = "wh-secret"
|
||||||
|
HEAD = "e" * 40
|
||||||
|
|
||||||
|
|
||||||
|
# --- unit: the signature gate ------------------------------------------------
|
||||||
|
|
||||||
|
def _sign(body: bytes, secret: str = SECRET) -> str:
|
||||||
|
return hmac.new(secret.encode(), body, hashlib.sha256).hexdigest()
|
||||||
|
|
||||||
|
|
||||||
|
def test_signature_gate():
|
||||||
|
body = b'{"x": 1}'
|
||||||
|
assert signature_ok(SECRET, body, _sign(body)) is True
|
||||||
|
assert signature_ok(SECRET, body, _sign(body).upper()) is True # hex case
|
||||||
|
assert signature_ok(SECRET, body, _sign(body, "wrong")) is False
|
||||||
|
assert signature_ok(SECRET, body, "") is False
|
||||||
|
assert signature_ok("", body, _sign(body)) is False
|
||||||
|
|
||||||
|
|
||||||
|
# --- unit: payload parsing ---------------------------------------------------
|
||||||
|
|
||||||
|
def test_push_facts_collects_and_dedups_paths():
|
||||||
|
payload = {
|
||||||
|
"after": HEAD,
|
||||||
|
"repository": {"clone_url": "https://git.example.com/alice/widget.git"},
|
||||||
|
"commits": [
|
||||||
|
{"added": ["a.py"], "modified": ["b.py"], "removed": []},
|
||||||
|
{"added": [], "modified": ["b.py", "c/d.py"], "removed": ["gone.py"]},
|
||||||
|
],
|
||||||
|
}
|
||||||
|
raw, changed, removed, head = push_facts(payload)
|
||||||
|
assert raw.endswith("alice/widget.git")
|
||||||
|
assert changed == ["a.py", "b.py", "c/d.py"]
|
||||||
|
assert removed == ["gone.py"]
|
||||||
|
assert head == HEAD
|
||||||
|
|
||||||
|
|
||||||
|
def test_push_facts_tolerates_an_empty_payload():
|
||||||
|
assert push_facts({}) == ("", [], [], "")
|
||||||
|
|
||||||
|
|
||||||
|
def test_path_touches_uses_the_location_semantics():
|
||||||
|
assert _path_touches("src/x.py", "src/x.py")
|
||||||
|
assert _path_touches("src", "src/lib/x.py") # recorded dir, file below
|
||||||
|
assert not _path_touches("src/x.py", "src/x_test.py")
|
||||||
|
assert not _path_touches("src/lib", "src/library/x.py") # no prefix bleed
|
||||||
|
assert not _path_touches("", "src/x.py")
|
||||||
|
|
||||||
|
|
||||||
|
def test_route_is_registered_and_unauthenticated_by_design():
|
||||||
|
from scribe.app import create_app
|
||||||
|
from scribe.routes import webhooks as wh
|
||||||
|
|
||||||
|
assert callable(wh.forge_push)
|
||||||
|
rules = {r.rule for r in create_app().url_map.iter_rules()}
|
||||||
|
assert "/api/webhooks/forge" in rules
|
||||||
|
|
||||||
|
|
||||||
|
# --- integration: the flag lands and clears on real Postgres -----------------
|
||||||
|
|
||||||
|
@pytest_asyncio.fixture
|
||||||
|
async def _dispose_engine():
|
||||||
|
from scribe.models import engine
|
||||||
|
yield
|
||||||
|
await engine.dispose()
|
||||||
|
|
||||||
|
|
||||||
|
@pytest_asyncio.fixture
|
||||||
|
async def seeded(_dispose_engine):
|
||||||
|
"""User + project + binding + two verified snippets + one unverified."""
|
||||||
|
from sqlalchemy import select
|
||||||
|
|
||||||
|
from scribe.models import async_session
|
||||||
|
from scribe.models.project import Project
|
||||||
|
from scribe.models.user import User
|
||||||
|
from scribe.services import snippets as svc
|
||||||
|
from scribe.services.repo_bindings import set_binding
|
||||||
|
|
||||||
|
async with async_session() as s:
|
||||||
|
user = (
|
||||||
|
await s.execute(select(User).where(User.username == "webhook_itest"))
|
||||||
|
).scalar_one_or_none()
|
||||||
|
if user is None:
|
||||||
|
user = User(username="webhook_itest")
|
||||||
|
s.add(user)
|
||||||
|
await s.flush()
|
||||||
|
project = Project(user_id=user.id, title="Widget")
|
||||||
|
s.add(project)
|
||||||
|
await s.flush()
|
||||||
|
uid, pid = user.id, project.id
|
||||||
|
await s.commit()
|
||||||
|
|
||||||
|
await set_binding(uid, "https://git.example.com/alice/widget.git", pid)
|
||||||
|
|
||||||
|
async def _snippet(name, path, code, verify=True):
|
||||||
|
note = await svc.create_snippet(
|
||||||
|
uid, name=name, code=code, language="python",
|
||||||
|
repo="Widget", path=path, symbol=name, project_id=pid,
|
||||||
|
)
|
||||||
|
if verify:
|
||||||
|
await svc.record_verification(uid, note.id, status="ok")
|
||||||
|
return note.id
|
||||||
|
|
||||||
|
return {
|
||||||
|
"uid": uid,
|
||||||
|
"hit": await _snippet("wh_hit", "src/x.py", "def wh_hit():\n return 1\n"),
|
||||||
|
"miss": await _snippet("wh_miss", "src/other.py", "def wh_miss():\n return 2\n"),
|
||||||
|
"gone": await _snippet("wh_gone", "src/gone.py", "def wh_gone():\n return 3\n"),
|
||||||
|
"unchecked": await _snippet(
|
||||||
|
"wh_unchecked", "src/x.py", "def wh_unchecked():\n return 4\n",
|
||||||
|
verify=False,
|
||||||
|
),
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.integration
|
||||||
|
async def test_push_flags_matched_verdicts_and_replay_is_quiet(seeded):
|
||||||
|
from scribe.services import snippets as svc
|
||||||
|
|
||||||
|
flagged = await svc.invalidate_for_push(
|
||||||
|
"git.example.com/alice/widget",
|
||||||
|
changed=["src/x.py"], removed=["src/gone.py"], commit_sha=HEAD,
|
||||||
|
)
|
||||||
|
# wh_hit (modified) + wh_gone (removed). wh_miss untouched; wh_unchecked
|
||||||
|
# carries no verdict and is skipped by design.
|
||||||
|
assert flagged == 2
|
||||||
|
|
||||||
|
uid = seeded["uid"]
|
||||||
|
hit = svc.snippet_to_dict(await svc.get_snippet(uid, seeded["hit"]))
|
||||||
|
assert hit["verification"]["needs_attention"] is True
|
||||||
|
assert hit["verification"]["invalidated_by"]["commit_sha"] == HEAD
|
||||||
|
assert hit["verification"]["invalidated_by"]["removed"] is False
|
||||||
|
|
||||||
|
gone = svc.snippet_to_dict(await svc.get_snippet(uid, seeded["gone"]))
|
||||||
|
assert gone["verification"]["invalidated_by"]["removed"] is True
|
||||||
|
|
||||||
|
miss = svc.snippet_to_dict(await svc.get_snippet(uid, seeded["miss"]))
|
||||||
|
assert miss["verification"]["needs_attention"] is False
|
||||||
|
|
||||||
|
unchecked = svc.snippet_to_dict(await svc.get_snippet(uid, seeded["unchecked"]))
|
||||||
|
assert unchecked["verification"]["status"] == "unverified"
|
||||||
|
|
||||||
|
# The attention listing — the operator's single entry point — now shows
|
||||||
|
# exactly the flagged pair, through the SQL dialect.
|
||||||
|
items, total = await svc.list_snippets(uid, verification="attention")
|
||||||
|
ids = {i["id"] for i in items}
|
||||||
|
assert {seeded["hit"], seeded["gone"]} <= ids
|
||||||
|
assert seeded["miss"] not in ids
|
||||||
|
|
||||||
|
# Replayed delivery: same head commit flags nothing new.
|
||||||
|
again = await svc.invalidate_for_push(
|
||||||
|
"git.example.com/alice/widget",
|
||||||
|
changed=["src/x.py"], removed=["src/gone.py"], commit_sha=HEAD,
|
||||||
|
)
|
||||||
|
assert again == 0
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.integration
|
||||||
|
async def test_reverifying_clears_the_flag(seeded):
|
||||||
|
from scribe.services import snippets as svc
|
||||||
|
|
||||||
|
await svc.invalidate_for_push(
|
||||||
|
"git.example.com/alice/widget", changed=["src/x.py"], removed=[],
|
||||||
|
commit_sha=HEAD,
|
||||||
|
)
|
||||||
|
uid = seeded["uid"]
|
||||||
|
await svc.record_verification(
|
||||||
|
uid, seeded["hit"], status="ok", detail="rechecked after push",
|
||||||
|
commit_sha=HEAD,
|
||||||
|
)
|
||||||
|
hit = svc.snippet_to_dict(await svc.get_snippet(uid, seeded["hit"]))
|
||||||
|
assert hit["verification"]["needs_attention"] is False
|
||||||
|
assert hit["verification"]["invalidated_by"] is None
|
||||||
|
items, _ = await svc.list_snippets(uid, verification="attention")
|
||||||
|
assert seeded["hit"] not in {i["id"] for i in items}
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.integration
|
||||||
|
async def test_unbound_repo_flags_nothing(seeded):
|
||||||
|
from scribe.services import snippets as svc
|
||||||
|
|
||||||
|
flagged = await svc.invalidate_for_push(
|
||||||
|
"github.com/somebody/else", changed=["src/x.py"], removed=[],
|
||||||
|
commit_sha=HEAD,
|
||||||
|
)
|
||||||
|
assert flagged == 0
|
||||||
@@ -170,6 +170,38 @@ def test_python_dialect_on_a_row_with_no_data_at_all():
|
|||||||
assert knowledge_svc.verification_matches(None, "ok") is False
|
assert knowledge_svc.verification_matches(None, "ok") is False
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize(
|
||||||
|
"value, expected",
|
||||||
|
[("ok", False), ("attention", True), ("drifted", False), ("unverified", False)],
|
||||||
|
)
|
||||||
|
def test_python_dialect_on_a_push_invalidated_ok_verdict(value, expected):
|
||||||
|
"""#2691: a push touched the recorded location since the verdict. Like the
|
||||||
|
expired case, it is neither drifted (nothing found wrong) nor unverified (a
|
||||||
|
check happened) — but the repo moved under the blessing, so `attention`
|
||||||
|
must include it and `ok` must not."""
|
||||||
|
data = _data("ok", "aaa", "aaa")
|
||||||
|
data["verification"]["invalidated_by"] = {"commit_sha": "d" * 40, "at": "t"}
|
||||||
|
assert knowledge_svc.verification_matches(data, value) is expected
|
||||||
|
|
||||||
|
|
||||||
|
def test_push_invalidation_reads_as_attention_and_clears_on_reverify():
|
||||||
|
"""The flag rides the verdict dict, so recording ANY fresh verdict clears
|
||||||
|
it by construction — compose_verification builds a new dict. No clearing
|
||||||
|
branch exists to forget."""
|
||||||
|
sha = code_sha("def f(): pass")
|
||||||
|
verdict = compose_verification(status="ok", checked_code_sha=sha)
|
||||||
|
verdict["invalidated_by"] = {"commit_sha": "d" * 40, "at": "t"}
|
||||||
|
fields = {"code": "def f(): pass", "verification": verdict}
|
||||||
|
view = verification_view(_note(), fields)
|
||||||
|
assert view["needs_attention"] is True
|
||||||
|
assert view["invalidated_by"]["commit_sha"] == "d" * 40
|
||||||
|
|
||||||
|
fresh = compose_verification(status="ok", checked_code_sha=sha)
|
||||||
|
assert "invalidated_by" not in fresh
|
||||||
|
view2 = verification_view(_note(), {"code": "def f(): pass", "verification": fresh})
|
||||||
|
assert view2["needs_attention"] is False
|
||||||
|
|
||||||
|
|
||||||
# --- the filter, SQL dialect ----------------------------------------------
|
# --- the filter, SQL dialect ----------------------------------------------
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user