feat(forge): GitHub adapter — second implementation keeps the seam a contract (#2693, milestone 288 step 8)
CI & Build / Python lint (push) Successful in 4s
CI & Build / Plugin hooks (push) Successful in 12s
CI & Build / integration (push) Successful in 27s
CI & Build / TypeScript typecheck (push) Successful in 34s
CI & Build / Python tests (push) Successful in 1m8s
CI & Build / Build & push image (push) Successful in 45s
CI & Build / Python lint (push) Successful in 4s
CI & Build / Plugin hooks (push) Successful in 12s
CI & Build / integration (push) Successful in 27s
CI & Build / TypeScript typecheck (push) Successful in 34s
CI & Build / Python tests (push) Successful in 1m8s
CI & Build / Build & push image (push) Successful in 45s
ForgeAdapter is now a named base class carrying the shared plumbing (host join, error taxonomy, contents decoding, archive, default_branch, latest_commit); GiteaForge keeps its exact behavior and GitHubForge joins with the real differences: api.github.com / GHE /api/v3 host mapping, Bearer auth, a commits call for the provenance stamp (GitHub's contents payload only carries the blob sha), and the codeload tarball redirect. The contract grew latest_commit, and with it the cached-SHA short-circuit in pull-time freshness: a stored provenance commit that still heads the recorded path confirms 'current' without a content transfer — the economy that fits pulls inside GitHub's rate limits; every surprise falls back to the full fetch. Webhook deliveries now also accept X-Hub-Signature-256 (sha256=<hex>); the payload shape was already common. Settings card copy covers both forges' token scopes; the kind selector already flowed from the server. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -2160,9 +2160,10 @@ function formatUserDate(iso: string): string {
|
||||
<section class="settings-section full-width">
|
||||
<h2>Git Forge</h2>
|
||||
<p class="section-desc">
|
||||
Optional read-only connection to your git forge (Gitea) so snippet
|
||||
code can be fetched and drift-checked server-side. A read-scope API
|
||||
token is enough. Leave the kind unset to keep the integration off.
|
||||
Optional read-only connection to your git forge (Gitea or GitHub) so
|
||||
snippet code can be fetched, drift-checked, and coverage-measured
|
||||
server-side. A read-scope token is enough. Leave the kind unset to
|
||||
keep the integration off.
|
||||
</p>
|
||||
<div class="smtp-grid">
|
||||
<div class="field">
|
||||
@@ -2179,6 +2180,13 @@ function formatUserDate(iso: string): string {
|
||||
<div class="field">
|
||||
<label for="forge-token">API Token (read scope)</label>
|
||||
<input id="forge-token" v-model="forge.token" type="password" class="input" />
|
||||
<p class="field-hint">
|
||||
Gitea: an access token with read scope on repositories. GitHub:
|
||||
a fine-grained PAT with Contents: Read-only (or a classic token
|
||||
with repo read). For GitHub, use
|
||||
<code>https://github.com</code> as the base URL — or your
|
||||
GitHub Enterprise instance's URL.
|
||||
</p>
|
||||
</div>
|
||||
<div class="field">
|
||||
<label for="forge-webhook-secret">Webhook Secret</label>
|
||||
|
||||
@@ -43,14 +43,24 @@ 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."""
|
||||
"""Validate a push signature: the hex HMAC-SHA256 of the raw body under
|
||||
the webhook secret (Gitea's X-Gitea-Signature verbatim; GitHub's
|
||||
X-Hub-Signature-256 minus its "sha256=" prefix). 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 delivered_signature(headers) -> str:
|
||||
"""The HMAC hex a delivery carries, whichever forge sent it: Gitea's
|
||||
X-Gitea-Signature verbatim, or GitHub's X-Hub-Signature-256 minus its
|
||||
"sha256=" scheme prefix (#2693). Empty when neither header is present."""
|
||||
return headers.get("X-Gitea-Signature", "") or headers.get(
|
||||
"X-Hub-Signature-256", ""
|
||||
).removeprefix("sha256=")
|
||||
|
||||
|
||||
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."""
|
||||
@@ -76,7 +86,10 @@ async def forge_push():
|
||||
return jsonify({"error": "Not found"}), 404
|
||||
|
||||
body = await request.get_data()
|
||||
if not signature_ok(secret, body, request.headers.get("X-Gitea-Signature", "")):
|
||||
# The payload shape push_facts reads (repository.clone_url,
|
||||
# commits[].added/modified/removed, after) is common to both forges, so
|
||||
# the signature header is the whole GitHub mapping.
|
||||
if not signature_ok(secret, body, delivered_signature(request.headers)):
|
||||
return jsonify({"error": "Invalid signature"}), 401
|
||||
|
||||
try:
|
||||
|
||||
@@ -33,7 +33,7 @@ import re
|
||||
import tarfile
|
||||
from datetime import datetime, timezone
|
||||
|
||||
from scribe.services.forge import GiteaForge, get_forge
|
||||
from scribe.services.forge import ForgeAdapter, get_forge
|
||||
from scribe.services.repo_bindings import keys_for_project
|
||||
from scribe.services.settings import get_setting, set_setting
|
||||
|
||||
@@ -252,7 +252,7 @@ async def _recorded_locations(user_id: int, project_id: int) -> list[tuple[str,
|
||||
|
||||
|
||||
async def compute_coverage(
|
||||
user_id: int, project_id: int, *, forge: GiteaForge | None = None
|
||||
user_id: int, project_id: int, *, forge: ForgeAdapter | None = None
|
||||
) -> dict | None:
|
||||
"""Measure a project's pattern-library coverage against its bound repos.
|
||||
|
||||
@@ -299,7 +299,7 @@ async def compute_coverage(
|
||||
|
||||
|
||||
async def refresh_coverage(
|
||||
user_id: int, project_id: int, *, forge: GiteaForge | None = None
|
||||
user_id: int, project_id: int, *, forge: ForgeAdapter | None = None
|
||||
) -> dict | None:
|
||||
"""Compute and cache. The only writer of the cache key."""
|
||||
coverage = await compute_coverage(user_id, project_id, forge=forge)
|
||||
|
||||
+185
-47
@@ -16,8 +16,9 @@ Design constraints, in force everywhere below:
|
||||
method to misuse. The token an operator mints for it only ever needs read
|
||||
scope, and the docs say so.
|
||||
- The contract stays as small as its consumers (steps 5-7): read_file /
|
||||
default_branch / resolve_repo / check. GitHub later implements this same
|
||||
contract (step 8); resist widening it speculatively.
|
||||
latest_commit / archive / default_branch / resolve_repo / check. Two
|
||||
implementations (Gitea, GitHub — step 8) keep it honest; resist widening
|
||||
it speculatively.
|
||||
- Repo identity is the repo-binding key — `normalize_repo_key`'s
|
||||
host/owner/repo — so the join between a snippet's recorded repo and the
|
||||
forge needs no new identity scheme. The host segment selects whether THIS
|
||||
@@ -50,8 +51,8 @@ FORGE_KIND_KEY = "forge_kind"
|
||||
FORGE_BASE_URL_KEY = "forge_base_url"
|
||||
FORGE_TOKEN_KEY = "forge_token"
|
||||
|
||||
# Kinds an instance can configure. GitHub joins in step 8 of milestone 288.
|
||||
FORGE_KINDS = ("gitea",)
|
||||
# Kinds an instance can configure. Matches _FORGE_CLASSES below.
|
||||
FORGE_KINDS = ("gitea", "github")
|
||||
|
||||
# Total budget per forge call. Consumers either have a cache to fall back to
|
||||
# (step 5) or a user watching a button (the test probe) — neither tolerates a
|
||||
@@ -89,15 +90,19 @@ def _host_of(url: str) -> str:
|
||||
return (urlsplit(url).hostname or "").lower()
|
||||
|
||||
|
||||
class GiteaForge:
|
||||
"""The Gitea implementation of the forge contract, over its REST API.
|
||||
class ForgeAdapter:
|
||||
"""The shared plumbing of the forge contract; adapters supply the API
|
||||
base, auth headers, and any endpoint that differs.
|
||||
|
||||
`transport` exists for tests: httpx.MockTransport makes the contract
|
||||
testable without a live server or a new dependency. Production callers
|
||||
never pass it.
|
||||
"""
|
||||
|
||||
kind = "gitea"
|
||||
kind = ""
|
||||
# One "newest commit for this path" page — the endpoint is shared but the
|
||||
# page-size parameter is not, so each adapter names its own.
|
||||
_commit_page_params: dict = {}
|
||||
|
||||
def __init__(self, base_url: str, token: str, *, transport=None) -> None:
|
||||
self.base_url = (base_url or "").rstrip("/")
|
||||
@@ -125,11 +130,22 @@ class GiteaForge:
|
||||
return None
|
||||
return rest
|
||||
|
||||
def _api_base(self) -> str:
|
||||
raise NotImplementedError
|
||||
|
||||
def _headers(self) -> dict:
|
||||
raise NotImplementedError
|
||||
|
||||
def _client(self) -> httpx.AsyncClient:
|
||||
kwargs: dict = {
|
||||
"base_url": f"{self.base_url}/api/v1",
|
||||
"headers": {"Authorization": f"token {self._token}"},
|
||||
"base_url": self._api_base(),
|
||||
"headers": self._headers(),
|
||||
"timeout": _TIMEOUT,
|
||||
# GitHub serves tarballs via a 302 to codeload. httpx drops the
|
||||
# Authorization header on the cross-host hop, and GitHub's
|
||||
# redirect target carries its own short-lived token in the URL —
|
||||
# so following is both necessary there and harmless on Gitea.
|
||||
"follow_redirects": True,
|
||||
}
|
||||
if self._transport is not None:
|
||||
kwargs["transport"] = self._transport
|
||||
@@ -150,6 +166,87 @@ class GiteaForge:
|
||||
raise ForgeError(f"forge returned HTTP {resp.status_code} for {url}")
|
||||
return resp
|
||||
|
||||
def _decode_contents(self, payload, path: str) -> str:
|
||||
"""Both forges speak the same contents-API dialect: a base64 file
|
||||
object, a list for a directory."""
|
||||
if isinstance(payload, list):
|
||||
raise ForgeNotFound(f"{path} is a directory on the forge, not a file")
|
||||
if payload.get("type") != "file":
|
||||
raise ForgeNotFound(
|
||||
f"{path} is a {payload.get('type', 'non-file')} on the forge"
|
||||
)
|
||||
if payload.get("encoding") != "base64" or payload.get("content") is None:
|
||||
raise ForgeError(f"forge returned no readable content for {path}")
|
||||
try:
|
||||
return base64.b64decode(payload["content"]).decode("utf-8")
|
||||
except (binascii.Error, UnicodeDecodeError) as exc:
|
||||
raise ForgeError(f"forge content for {path} is not utf-8 text") from exc
|
||||
|
||||
async def _newest_commit(
|
||||
self, client: httpx.AsyncClient, repo: str, path: str, ref: str
|
||||
) -> str:
|
||||
params: dict = {**self._commit_page_params, "path": path}
|
||||
if ref:
|
||||
params["sha"] = ref
|
||||
resp = await self._get(client, f"/repos/{repo}/commits", params=params)
|
||||
payload = resp.json()
|
||||
# Tolerant parse on purpose: the caller uses this as an optimization
|
||||
# and falls back to read_file, so a surprising payload must read as
|
||||
# "don't know", never break a pull.
|
||||
if isinstance(payload, list) and payload and isinstance(payload[0], dict):
|
||||
return str(payload[0].get("sha") or "")
|
||||
return ""
|
||||
|
||||
async def latest_commit(self, repo: str, path: str, ref: str = "") -> str:
|
||||
"""The newest commit touching ``path`` — "" when it can't be told.
|
||||
|
||||
The cached-SHA short-circuit (#2693): when a snippet's provenance
|
||||
already names a commit, this one small call can prove the file
|
||||
hasn't moved since — no content transfer, which is what keeps
|
||||
pull-time freshness inside GitHub's rate limits.
|
||||
"""
|
||||
async with self._client() as client:
|
||||
return await self._newest_commit(client, repo, path, ref)
|
||||
|
||||
def _archive_url(self, repo: str, ref: str) -> str:
|
||||
raise NotImplementedError
|
||||
|
||||
async def archive(self, repo: str, ref: str) -> bytes:
|
||||
"""The repo's content at ``ref`` as a gzipped tarball, in one request.
|
||||
|
||||
Coverage measurement (step 7) needs every source file's text; per-file
|
||||
reads would mean one API call per file, so the archive endpoint is the
|
||||
only shape that scales past toy repos. Callers must never run this in
|
||||
a request path — it moves the whole repo.
|
||||
"""
|
||||
async with self._client() as client:
|
||||
resp = await self._get(
|
||||
client, self._archive_url(repo, ref), timeout=_ARCHIVE_TIMEOUT
|
||||
)
|
||||
return resp.content
|
||||
|
||||
async def default_branch(self, repo: str) -> str:
|
||||
async with self._client() as client:
|
||||
resp = await self._get(client, f"/repos/{repo}")
|
||||
branch = (resp.json() or {}).get("default_branch") or ""
|
||||
if not branch:
|
||||
raise ForgeError(f"forge reported no default branch for {repo}")
|
||||
return branch
|
||||
|
||||
|
||||
class GiteaForge(ForgeAdapter):
|
||||
"""The Gitea implementation of the forge contract, over its REST API."""
|
||||
|
||||
kind = "gitea"
|
||||
# stat/verification/files add per-commit work Gitea skips when told to.
|
||||
_commit_page_params = {"limit": 1, "stat": "false"}
|
||||
|
||||
def _api_base(self) -> str:
|
||||
return f"{self.base_url}/api/v1"
|
||||
|
||||
def _headers(self) -> dict:
|
||||
return {"Authorization": f"token {self._token}"}
|
||||
|
||||
async def read_file(self, repo: str, path: str, ref: str = "") -> ForgeFile:
|
||||
"""Read one file's current content, with the commit it was served at.
|
||||
|
||||
@@ -164,18 +261,7 @@ class GiteaForge:
|
||||
params=params,
|
||||
)
|
||||
payload = resp.json()
|
||||
if isinstance(payload, list):
|
||||
raise ForgeNotFound(f"{path} is a directory on the forge, not a file")
|
||||
if payload.get("type") != "file":
|
||||
raise ForgeNotFound(
|
||||
f"{path} is a {payload.get('type', 'non-file')} on the forge"
|
||||
)
|
||||
if payload.get("encoding") != "base64" or payload.get("content") is None:
|
||||
raise ForgeError(f"forge returned no readable content for {path}")
|
||||
try:
|
||||
content = base64.b64decode(payload["content"]).decode("utf-8")
|
||||
except (binascii.Error, UnicodeDecodeError) as exc:
|
||||
raise ForgeError(f"forge content for {path} is not utf-8 text") from exc
|
||||
content = self._decode_contents(payload, path)
|
||||
return ForgeFile(
|
||||
content=content,
|
||||
# last_commit_sha is the commit that last touched the file — the
|
||||
@@ -185,29 +271,8 @@ class GiteaForge:
|
||||
path=payload.get("path") or path,
|
||||
)
|
||||
|
||||
async def archive(self, repo: str, ref: str) -> bytes:
|
||||
"""The repo's content at ``ref`` as a gzipped tarball, in one request.
|
||||
|
||||
Coverage measurement (step 7) needs every source file's text; per-file
|
||||
reads would mean one API call per file, so the archive endpoint is the
|
||||
only shape that scales past toy repos. Callers must never run this in
|
||||
a request path — it moves the whole repo.
|
||||
"""
|
||||
async with self._client() as client:
|
||||
resp = await self._get(
|
||||
client,
|
||||
f"/repos/{repo}/archive/{quote(ref, safe='')}.tar.gz",
|
||||
timeout=_ARCHIVE_TIMEOUT,
|
||||
)
|
||||
return resp.content
|
||||
|
||||
async def default_branch(self, repo: str) -> str:
|
||||
async with self._client() as client:
|
||||
resp = await self._get(client, f"/repos/{repo}")
|
||||
branch = (resp.json() or {}).get("default_branch") or ""
|
||||
if not branch:
|
||||
raise ForgeError(f"forge reported no default branch for {repo}")
|
||||
return branch
|
||||
def _archive_url(self, repo: str, ref: str) -> str:
|
||||
return f"/repos/{repo}/archive/{quote(ref, safe='')}.tar.gz"
|
||||
|
||||
async def check(self) -> dict:
|
||||
"""Health probe for the settings test button: reach the forge AND
|
||||
@@ -222,6 +287,78 @@ class GiteaForge:
|
||||
}
|
||||
|
||||
|
||||
class GitHubForge(ForgeAdapter):
|
||||
"""The GitHub implementation — the second one, which is the point (#2693):
|
||||
it proves the seam is a contract rather than a Gitea-shaped hole. Works
|
||||
against github.com and GitHub Enterprise; the token is a fine-grained PAT
|
||||
with Contents: Read-only (or a classic token with `repo` read)."""
|
||||
|
||||
kind = "github"
|
||||
_commit_page_params = {"per_page": 1}
|
||||
|
||||
_API_VERSION = "2022-11-28"
|
||||
|
||||
def _api_base(self) -> str:
|
||||
# github.com's API lives on its own host; GitHub Enterprise serves
|
||||
# the same API under the instance at /api/v3.
|
||||
if self.host == "github.com":
|
||||
return "https://api.github.com"
|
||||
return f"{self.base_url}/api/v3"
|
||||
|
||||
def _headers(self) -> dict:
|
||||
return {
|
||||
"Authorization": f"Bearer {self._token}",
|
||||
"Accept": "application/vnd.github+json",
|
||||
"X-GitHub-Api-Version": self._API_VERSION,
|
||||
}
|
||||
|
||||
async def read_file(self, repo: str, path: str, ref: str = "") -> ForgeFile:
|
||||
"""Same contents-API dialect as Gitea, minus one field: GitHub's
|
||||
payload carries only the blob sha — a content address, not a point in
|
||||
history — so the provenance stamp costs one extra commits call. ""
|
||||
when even that can't be told; consumers already treat an empty stamp
|
||||
as "don't restamp"."""
|
||||
params = {"ref": ref} if ref else None
|
||||
async with self._client() as client:
|
||||
resp = await self._get(
|
||||
client,
|
||||
f"/repos/{repo}/contents/{quote(path, safe='/')}",
|
||||
params=params,
|
||||
)
|
||||
payload = resp.json()
|
||||
content = self._decode_contents(payload, path)
|
||||
try:
|
||||
commit_sha = await self._newest_commit(client, repo, path, ref)
|
||||
except ForgeError:
|
||||
commit_sha = ""
|
||||
return ForgeFile(
|
||||
content=content,
|
||||
commit_sha=commit_sha,
|
||||
path=payload.get("path") or path,
|
||||
)
|
||||
|
||||
def _archive_url(self, repo: str, ref: str) -> str:
|
||||
return f"/repos/{repo}/tarball/{quote(ref, safe='')}"
|
||||
|
||||
async def check(self) -> dict:
|
||||
"""GitHub has no /version endpoint; proving the token against /user
|
||||
is the whole probe, and the pinned API version stands in as the
|
||||
version string."""
|
||||
async with self._client() as client:
|
||||
user = (await self._get(client, "/user")).json() or {}
|
||||
return {
|
||||
"ok": True,
|
||||
"version": f"GitHub API {self._API_VERSION}",
|
||||
"username": user.get("login") or "",
|
||||
}
|
||||
|
||||
|
||||
_FORGE_CLASSES: dict[str, type[ForgeAdapter]] = {
|
||||
"gitea": GiteaForge,
|
||||
"github": GitHubForge,
|
||||
}
|
||||
|
||||
|
||||
async def forge_config() -> dict:
|
||||
"""The instance's forge configuration, DB-first with env fallback.
|
||||
|
||||
@@ -241,11 +378,12 @@ async def forge_config() -> dict:
|
||||
}
|
||||
|
||||
|
||||
async def get_forge(*, transport=None) -> GiteaForge | None:
|
||||
async def get_forge(*, transport=None) -> ForgeAdapter | None:
|
||||
"""The configured forge adapter, or None — and None means "behave exactly
|
||||
as if this module did not exist", which every consumer must honor."""
|
||||
cfg = await forge_config()
|
||||
if cfg["kind"] not in FORGE_KINDS:
|
||||
cls = _FORGE_CLASSES.get(cfg["kind"])
|
||||
if cls is None:
|
||||
if cfg["kind"]:
|
||||
# A kind we don't implement is a misconfiguration, not "off" —
|
||||
# say so once per lookup rather than silently reading as absent.
|
||||
@@ -256,4 +394,4 @@ async def get_forge(*, transport=None) -> GiteaForge | None:
|
||||
if not cfg["base_url"].startswith(("http://", "https://")):
|
||||
logger.warning("forge base URL %r has no http(s) scheme — forge disabled", cfg["base_url"])
|
||||
return None
|
||||
return GiteaForge(cfg["base_url"], cfg["token"], transport=transport)
|
||||
return cls(cfg["base_url"], cfg["token"], transport=transport)
|
||||
|
||||
@@ -1046,10 +1046,27 @@ async def attach_live_body(note, data: dict) -> None:
|
||||
data["body_freshness"] = "repo-not-on-this-forge"
|
||||
return
|
||||
|
||||
stored_prov_sha = (fields.get("provenance") or {}).get("commit_sha") or ""
|
||||
|
||||
async def _probe():
|
||||
# Cached-SHA short-circuit (#2693): provenance names the commit the
|
||||
# cached code was last confirmed at, so one cheap "newest commit
|
||||
# touching this path" call can prove the file hasn't moved since —
|
||||
# no content transfer. That economy is what fits pull-time freshness
|
||||
# inside GitHub's rate limits; it's merely nice on a self-hosted
|
||||
# Gitea. Any surprise (error, empty, mismatch) falls through to the
|
||||
# full fetch, which stays the authoritative path.
|
||||
if stored_prov_sha:
|
||||
try:
|
||||
head = await forge.latest_commit(repo, loc["path"])
|
||||
except ForgeError:
|
||||
head = ""
|
||||
if head and head == stored_prov_sha:
|
||||
return None
|
||||
return await forge.read_file(repo, loc["path"])
|
||||
|
||||
try:
|
||||
fetched = await asyncio.wait_for(
|
||||
forge.read_file(repo, loc["path"]), timeout=PULL_FETCH_BUDGET_S
|
||||
)
|
||||
fetched = await asyncio.wait_for(_probe(), timeout=PULL_FETCH_BUDGET_S)
|
||||
except ForgeNotFound:
|
||||
data["body_source"] = "cache"
|
||||
data["body_freshness"] = "missing"
|
||||
@@ -1064,6 +1081,14 @@ async def attach_live_body(note, data: dict) -> None:
|
||||
data["body_freshness"] = "unreachable"
|
||||
return
|
||||
|
||||
if fetched is None:
|
||||
# Unchanged since the provenance commit — confirmed against the
|
||||
# source without moving the file. Same stamp, so nothing to persist
|
||||
# (the same-sha rule); the body already reflects that commit.
|
||||
data["body_source"] = "forge"
|
||||
data["body_freshness"] = "current"
|
||||
return
|
||||
|
||||
cached = _normalized_code(fields.get("code") or "")
|
||||
if cached and cached in _normalized_code(fetched.content):
|
||||
data["body_source"] = "forge"
|
||||
|
||||
@@ -19,7 +19,7 @@ import hmac
|
||||
import pytest
|
||||
import pytest_asyncio
|
||||
|
||||
from scribe.routes.webhooks import push_facts, signature_ok
|
||||
from scribe.routes.webhooks import delivered_signature, push_facts, signature_ok
|
||||
from scribe.services.snippets import _path_touches
|
||||
|
||||
SECRET = "wh-secret"
|
||||
@@ -41,6 +41,46 @@ def test_signature_gate():
|
||||
assert signature_ok("", body, _sign(body)) is False
|
||||
|
||||
|
||||
def test_delivered_signature_reads_both_forges_headers():
|
||||
"""#2693: GitHub signs the same HMAC but ships it as
|
||||
X-Hub-Signature-256: sha256=<hex> — the whole webhook payload mapping is
|
||||
this header, so pin it."""
|
||||
hexsig = _sign(b"{}")
|
||||
assert delivered_signature({"X-Gitea-Signature": hexsig}) == hexsig
|
||||
assert delivered_signature({"X-Hub-Signature-256": f"sha256={hexsig}"}) == hexsig
|
||||
# Gitea's header wins when both appear; absence reads as empty (→ 401).
|
||||
assert delivered_signature({}) == ""
|
||||
# The stripped GitHub form still passes the gate end to end.
|
||||
assert signature_ok(
|
||||
SECRET, b'{"x": 1}',
|
||||
delivered_signature({"X-Hub-Signature-256": "sha256=" + _sign(b'{"x": 1}')}),
|
||||
)
|
||||
|
||||
|
||||
def test_push_facts_reads_a_github_shaped_payload():
|
||||
"""GitHub's push payload carries the same fields push_facts consumes —
|
||||
asserted against a real-shaped sample so a rename on either side of the
|
||||
mapping breaks a test instead of silently flagging nothing."""
|
||||
payload = {
|
||||
"ref": "refs/heads/main",
|
||||
"after": HEAD,
|
||||
"repository": {
|
||||
"full_name": "alice/widget",
|
||||
"clone_url": "https://github.com/alice/widget.git",
|
||||
"html_url": "https://github.com/alice/widget",
|
||||
},
|
||||
"commits": [
|
||||
{"id": "a" * 40, "added": [], "modified": ["src/x.py"], "removed": []},
|
||||
],
|
||||
"head_commit": {"id": HEAD},
|
||||
}
|
||||
raw, changed, removed, head = push_facts(payload)
|
||||
assert raw == "https://github.com/alice/widget.git"
|
||||
assert changed == ["src/x.py"]
|
||||
assert removed == []
|
||||
assert head == HEAD
|
||||
|
||||
|
||||
# --- unit: payload parsing ---------------------------------------------------
|
||||
|
||||
def test_push_facts_collects_and_dedups_paths():
|
||||
|
||||
@@ -205,10 +205,19 @@ def test_forge_error_taxonomy_is_catchable_as_one_family():
|
||||
|
||||
|
||||
def test_adapter_contract_surface():
|
||||
"""Step 8's GitHub adapter implements exactly this surface — pin it."""
|
||||
for method in ("read_file", "default_branch", "resolve_repo", "check"):
|
||||
assert callable(getattr(GiteaForge, method))
|
||||
"""Both adapters implement exactly this surface — the second
|
||||
implementation is what proves it's a contract (#2693)."""
|
||||
from scribe.services.forge import FORGE_KINDS, GitHubForge
|
||||
|
||||
for cls in (GiteaForge, GitHubForge):
|
||||
for method in (
|
||||
"read_file", "latest_commit", "archive",
|
||||
"default_branch", "resolve_repo", "check",
|
||||
):
|
||||
assert callable(getattr(cls, method))
|
||||
assert GiteaForge.kind == "gitea"
|
||||
assert GitHubForge.kind == "github"
|
||||
assert set(FORGE_KINDS) == {"gitea", "github"}
|
||||
|
||||
|
||||
def test_admin_routes_registered():
|
||||
@@ -241,3 +250,122 @@ def test_config_has_the_docker_secret_channel():
|
||||
from scribe.config import Config
|
||||
for attr in ("FORGE_KIND", "FORGE_BASE_URL", "FORGE_TOKEN"):
|
||||
assert hasattr(Config, attr)
|
||||
|
||||
|
||||
# --- the GitHub adapter (#2693) ----------------------------------------------
|
||||
# Same contract, second implementation. Where behavior below differs from the
|
||||
# Gitea tests above, that difference IS the adapter's job: API host mapping,
|
||||
# Bearer auth, the missing last_commit_sha, the codeload redirect.
|
||||
|
||||
def _github(handler, base: str = "https://github.com"):
|
||||
from scribe.services.forge import GitHubForge
|
||||
|
||||
return GitHubForge(base, "gh-tok", transport=httpx.MockTransport(handler))
|
||||
|
||||
|
||||
def test_github_resolve_repo_is_the_same_host_join():
|
||||
from scribe.services.forge import GitHubForge
|
||||
|
||||
forge = GitHubForge("https://github.com", "t")
|
||||
assert forge.resolve_repo("git@github.com:alice/Widget.git") == "alice/widget"
|
||||
# A Gitea-hosted repo is a NORMAL miss for a GitHub forge, and vice versa.
|
||||
assert forge.resolve_repo("https://git.example.com/alice/widget") is None
|
||||
|
||||
|
||||
async def test_github_api_base_maps_dot_com_and_enterprise():
|
||||
seen = []
|
||||
|
||||
def handler(request):
|
||||
seen.append(str(request.url))
|
||||
return _json(200, {"default_branch": "main"})
|
||||
|
||||
await _github(handler).default_branch("alice/widget")
|
||||
await _github(handler, base="https://ghe.example.com").default_branch("alice/widget")
|
||||
assert seen[0] == "https://api.github.com/repos/alice/widget"
|
||||
assert seen[1] == "https://ghe.example.com/api/v3/repos/alice/widget"
|
||||
|
||||
|
||||
async def test_github_read_file_decodes_and_stamps_from_the_commits_call():
|
||||
content = "def canonical():\n return 1\n"
|
||||
|
||||
def handler(request):
|
||||
assert request.headers["Authorization"] == "Bearer gh-tok"
|
||||
assert request.headers["X-GitHub-Api-Version"]
|
||||
if request.url.path.endswith("/commits"):
|
||||
assert request.url.params["path"] == "src/x.py"
|
||||
assert request.url.params["per_page"] == "1"
|
||||
return _json(200, [{"sha": "c" * 40}])
|
||||
return _json(200, {
|
||||
"type": "file", "encoding": "base64",
|
||||
"content": base64.b64encode(content.encode()).decode(),
|
||||
"path": "src/x.py", "sha": "blob-sha-not-a-point-in-history",
|
||||
})
|
||||
|
||||
f = await _github(handler).read_file("alice/widget", "src/x.py")
|
||||
assert f.content == content
|
||||
# From /commits — GitHub's contents payload only carries the blob sha,
|
||||
# which is a content address, not the provenance stamp.
|
||||
assert f.commit_sha == "c" * 40
|
||||
|
||||
|
||||
async def test_github_read_file_serves_content_even_when_the_stamp_fails():
|
||||
content = "x = 1\n"
|
||||
|
||||
def handler(request):
|
||||
if request.url.path.endswith("/commits"):
|
||||
return httpx.Response(500)
|
||||
return _json(200, {"type": "file", "encoding": "base64",
|
||||
"content": base64.b64encode(content.encode()).decode()})
|
||||
|
||||
f = await _github(handler).read_file("alice/widget", "x.py")
|
||||
assert f.content == content
|
||||
assert f.commit_sha == "" # unknown stamp, not a failed read
|
||||
|
||||
|
||||
async def test_github_archive_follows_the_codeload_redirect():
|
||||
def handler(request):
|
||||
if request.url.host == "api.github.com":
|
||||
return httpx.Response(302, headers={
|
||||
"Location": "https://codeload.github.com/alice/widget/tar.gz/main",
|
||||
})
|
||||
assert request.url.host == "codeload.github.com"
|
||||
# httpx drops Authorization on the cross-host hop — codeload's URL
|
||||
# carries its own grant, and leaking the PAT there would be a bug.
|
||||
assert "Authorization" not in request.headers
|
||||
return httpx.Response(200, content=b"tarball-bytes")
|
||||
|
||||
assert await _github(handler).archive("alice/widget", "main") == b"tarball-bytes"
|
||||
|
||||
|
||||
async def test_github_check_probes_the_token_with_user():
|
||||
result = await _github(lambda r: _json(200, {"login": "octo"})).check()
|
||||
assert result["ok"] is True
|
||||
assert result["username"] == "octo"
|
||||
|
||||
|
||||
async def test_latest_commit_parses_tolerantly_on_both_adapters():
|
||||
"""The one caller treats latest_commit as an optimization with a fallback,
|
||||
so a surprising payload must read as "don't know", never raise."""
|
||||
assert await _github(
|
||||
lambda r: _json(200, [{"sha": "d" * 40}])
|
||||
).latest_commit("a/w", "x.py") == "d" * 40
|
||||
assert await _github(
|
||||
lambda r: _json(200, {"weird": True})
|
||||
).latest_commit("a/w", "x.py") == ""
|
||||
assert await _forge(
|
||||
lambda r: _json(200, [{"sha": "e" * 40}])
|
||||
).latest_commit("a/w", "x.py") == "e" * 40
|
||||
assert await _forge(lambda r: _json(200, [])).latest_commit("a/w", "x.py") == ""
|
||||
|
||||
|
||||
async def test_full_config_builds_a_github_adapter():
|
||||
from scribe.services.forge import GitHubForge
|
||||
|
||||
with _settings({
|
||||
"forge_kind": "github",
|
||||
"forge_base_url": "https://github.com",
|
||||
"forge_token": "tok",
|
||||
}), patch("scribe.services.forge.Config") as cfg:
|
||||
cfg.FORGE_KIND = cfg.FORGE_BASE_URL = cfg.FORGE_TOKEN = ""
|
||||
forge = await get_forge()
|
||||
assert isinstance(forge, GitHubForge)
|
||||
|
||||
@@ -104,6 +104,73 @@ async def test_current_with_same_stored_sha_skips_the_write():
|
||||
update.assert_not_called()
|
||||
|
||||
|
||||
async def test_stored_sha_short_circuit_skips_the_content_fetch():
|
||||
"""#2693: when provenance already names a commit and the forge reports no
|
||||
newer commit touching the path, the pull is confirmed current WITHOUT a
|
||||
content transfer — the economy that fits pull-time freshness inside
|
||||
GitHub's rate limits."""
|
||||
calls = []
|
||||
|
||||
def handler(request):
|
||||
calls.append(request.url.path)
|
||||
if request.url.path.endswith("/commits"):
|
||||
return httpx.Response(200, json=[{"sha": SHA}])
|
||||
raise AssertionError("the content fetch should have been skipped")
|
||||
|
||||
update = AsyncMock()
|
||||
data = _data(provenance={"commit_sha": SHA, "fetched_at": "t"})
|
||||
with _patched(_forge_with(handler)), patch.object(svc.notes_svc, "update_note", update):
|
||||
await svc.attach_live_body(_note(), data)
|
||||
await background.drain()
|
||||
assert data["body_source"] == "forge"
|
||||
assert data["body_freshness"] == "current"
|
||||
assert len(calls) == 1
|
||||
update.assert_not_called() # same stamp — nothing to persist
|
||||
|
||||
|
||||
async def test_moved_file_falls_through_to_the_full_fetch():
|
||||
new_sha = "0" * 40
|
||||
|
||||
def handler(request):
|
||||
if request.url.path.endswith("/commits"):
|
||||
return httpx.Response(200, json=[{"sha": new_sha}])
|
||||
return _file_response("prefix\n" + CODE, commit_sha=new_sha)
|
||||
|
||||
saved = {}
|
||||
|
||||
async def fake_update(uid, nid, **fields):
|
||||
saved.update(fields)
|
||||
|
||||
data = _data(provenance={"commit_sha": SHA, "fetched_at": "t"})
|
||||
with _patched(_forge_with(handler)), patch.object(
|
||||
svc.notes_svc, "update_note", fake_update
|
||||
):
|
||||
await svc.attach_live_body(_note(), data)
|
||||
await background.drain()
|
||||
# The file moved but still contains the code — current, with the stamp
|
||||
# advanced by the authoritative full fetch.
|
||||
assert data["body_freshness"] == "current"
|
||||
assert saved["data"]["provenance"]["commit_sha"] == new_sha
|
||||
|
||||
|
||||
async def test_short_circuit_failure_degrades_to_the_full_fetch():
|
||||
"""A forge whose commits endpoint errors must cost nothing: the full
|
||||
fetch stays the authoritative path and the pull behaves as before."""
|
||||
|
||||
def handler(request):
|
||||
if request.url.path.endswith("/commits"):
|
||||
return httpx.Response(500)
|
||||
return _file_response("prefix\n" + CODE)
|
||||
|
||||
update = AsyncMock()
|
||||
data = _data(provenance={"commit_sha": SHA, "fetched_at": "t"})
|
||||
with _patched(_forge_with(handler)), patch.object(svc.notes_svc, "update_note", update):
|
||||
await svc.attach_live_body(_note(), data)
|
||||
await background.drain()
|
||||
assert data["body_freshness"] == "current"
|
||||
update.assert_not_called() # same sha via the full fetch → same-sha skip
|
||||
|
||||
|
||||
async def test_diverged_reports_without_clobbering():
|
||||
forge = _forge_with(lambda r: _file_response("def helper(x):\n return x - 1\n"))
|
||||
update = AsyncMock()
|
||||
|
||||
Reference in New Issue
Block a user