fix(session): bind workspace-verified repository scope at first bind (#714)
The independent review reproduced a real hole: the production first-bind path never pinned repository/org, so the drift checks it feeds were skipped. Root cause ---------- gitea_whoami, gitea_get_runtime_context, gitea_resolve_task_capability and gitea_activate_profile all seeded the session context without repository or org. First-bind-wins then made the mutation gate's later seed a no-op, and assess_session_context only compares repository/org when the bound fields are non-null. Result, reproduced in production order with the real REMOTES: after whoami: repository=None org=None same-host repo=Other-Tools -> NOT BLOCKED same-host org=Other-Org -> NOT BLOCKED cross-host -> blocked (this part always worked) Binding REMOTES defaults would not fix it: REMOTES['prgs'].repo is 'Timesheet' (a default *target*, not an authorization scope, see #530), so pinning it fails closed on every legitimate Gitea-Tools mutation. There was no trusted source for repository scope, so this adds one. Design ------ - New optional profile field `allowed_repositories`: canonical owner/repository slugs. An authorization boundary, never the binding itself. Config-only — no env var can widen or forge it. - The session repository is derived from the verified, workspace-aligned git remote, never from caller-supplied org/repo, and validated against that allowlist. The session binds immutably to exactly one canonical slug; org is derived from it. A profile authorizing several repositories still binds to the single verified one. - Every first-bind entry point now pins the same complete context. Activation rejects an unauthorized/unverifiable workspace. Mutations fail closed when repository/org is unverified, and a tool-level org/repo override that disagrees with the binding is rejected before the write. - A mutation request can no longer establish, complete, or replace the binding (it previously seeded from `org or REMOTES[...]`, i.e. caller-controlled). - Enforcement is opt-in per profile: a profile without the field keeps prior behaviour, so existing static-profile namespaces stay functional. First-bind-wins, immutability, the RLock, profile isolation, host/identity validation and the private pytest-only reset are unchanged. gitea_auth.get_profile() built an explicit whitelist dict and silently dropped `allowed_repositories`; the new integration tests caught that the scope was never enforced through the real path. Tests ----- New tests/test_issue_714_production_first_bind.py drives the real entry points in production order rather than constructing _SessionContext directly. Covers complete first bind via each entry point, same-host repo/org drift, Timesheet and other-repo/other-org rejection, unverified and unauthorized workspaces, caller values unable to establish the binding, concurrent init selecting one repository, and the PR #715 author path staying functional. Mutation-verified: disabling trusted derivation, override validation, the scope check, or the get_profile passthrough each fails these tests (9/8/4/5 failures). No security assertion was weakened, removed, skipped, or rewritten. Focused: 26 passed. Issue #714 total: 46 passed. Prior failing modules: 240 passed. Config/profile/remote modules: 130 passed. Full suite: 2739 passed, 6 skipped, 1 pre-existing warning, 161 subtests in 25.80s. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
This commit is contained in:
+132
-1
@@ -11,6 +11,7 @@ substitution is forbidden.
|
||||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
import re
|
||||
import threading
|
||||
import urllib.parse
|
||||
from dataclasses import dataclass
|
||||
@@ -232,12 +233,16 @@ def assess_session_context(
|
||||
org: str | None = None,
|
||||
expected_username: str | None = None,
|
||||
require_bound: bool = False,
|
||||
require_complete: bool = False,
|
||||
) -> dict[str, Any]:
|
||||
"""Compare live values against the bound session context.
|
||||
|
||||
Returns ``proven`` / ``block`` / ``reasons``. When unbound and
|
||||
``require_bound`` is false, does not block (caller may seed). When
|
||||
unbound and ``require_bound`` is true, fails closed.
|
||||
unbound and ``require_bound`` is true, fails closed. ``require_complete``
|
||||
additionally fails closed when the binding carries no verified
|
||||
repository/organization identity, so a mutation can never run against an
|
||||
unknown repository (#714).
|
||||
"""
|
||||
reasons: list[str] = []
|
||||
with _SESSION_CONTEXT_LOCK:
|
||||
@@ -252,6 +257,15 @@ def assess_session_context(
|
||||
return _assessment(False, reasons, ctx)
|
||||
return _assessment(True, reasons, ctx)
|
||||
|
||||
if require_complete and not (ctx.get("repository") and ctx.get("org")):
|
||||
reasons.append(
|
||||
"session repository/organization identity is unverified "
|
||||
f"(repository={ctx.get('repository')!r}, org={ctx.get('org')!r}); "
|
||||
"a mutation cannot proceed without a verified workspace "
|
||||
"repository (fail closed)"
|
||||
)
|
||||
return _assessment(False, reasons, ctx)
|
||||
|
||||
live_profile = (profile_name or "").strip() or None
|
||||
live_remote = (remote or "").strip() or None
|
||||
live_host = (host or "").strip().lower() or None
|
||||
@@ -327,6 +341,123 @@ def assess_identity_match(
|
||||
}
|
||||
|
||||
|
||||
_REPO_SLUG_RE = re.compile(r"^\s*(?P<org>[^/\s]+)\s*/\s*(?P<repo>[^/\s]+?)(?:\.git)?\s*$")
|
||||
|
||||
|
||||
def parse_repository_slug(slug: str | None) -> tuple[str, str] | None:
|
||||
"""Split a canonical ``owner/repository`` slug, or None when unparseable."""
|
||||
match = _REPO_SLUG_RE.match(slug or "")
|
||||
if not match:
|
||||
return None
|
||||
return match.group("org"), match.group("repo")
|
||||
|
||||
|
||||
def format_repository_slug(org: str | None, repo: str | None) -> str | None:
|
||||
"""``owner/repository`` from parts, or None when either side is missing."""
|
||||
left = (org or "").strip()
|
||||
right = (repo or "").strip()
|
||||
if not left or not right:
|
||||
return None
|
||||
return f"{left}/{right}"
|
||||
|
||||
|
||||
def declared_allowed_repositories(profile: Mapping[str, Any] | None) -> list[str]:
|
||||
"""Canonical ``owner/repository`` authorization boundary declared by *profile*.
|
||||
|
||||
This list is an authorization boundary, never the session binding itself:
|
||||
the verified workspace selects exactly one entry (see
|
||||
:func:`assess_repository_scope`). Returns ``[]`` when the profile declares
|
||||
no scope — enforcement is opt-in per profile so existing static-profile
|
||||
namespaces stay functional until an operator provisions the field.
|
||||
"""
|
||||
if not profile:
|
||||
return []
|
||||
raw = profile.get("allowed_repositories")
|
||||
if not isinstance(raw, (list, tuple)):
|
||||
return []
|
||||
slugs: list[str] = []
|
||||
for entry in raw:
|
||||
if not isinstance(entry, str):
|
||||
continue
|
||||
parsed = parse_repository_slug(entry)
|
||||
if parsed:
|
||||
slugs.append(f"{parsed[0]}/{parsed[1]}")
|
||||
return slugs
|
||||
|
||||
|
||||
def assess_repository_scope(
|
||||
*,
|
||||
workspace_slug: str | None,
|
||||
allowed: list[str] | None,
|
||||
profile_name: str | None = None,
|
||||
) -> dict[str, Any]:
|
||||
"""Authorize the verified workspace repository against the profile allowlist.
|
||||
|
||||
The workspace-derived slug is the only candidate: a profile that authorizes
|
||||
several repositories still binds to the single verified one, and never to
|
||||
the list as a whole.
|
||||
"""
|
||||
scope = list(allowed or [])
|
||||
reasons: list[str] = []
|
||||
if not scope:
|
||||
return {
|
||||
"proven": True,
|
||||
"block": False,
|
||||
"reasons": reasons,
|
||||
"scope_enforced": False,
|
||||
"workspace_slug": workspace_slug,
|
||||
"allowed_repositories": [],
|
||||
}
|
||||
name = profile_name or "(active profile)"
|
||||
if not workspace_slug:
|
||||
reasons.append(
|
||||
f"no verified workspace repository could be established for profile "
|
||||
f"'{name}'; repository scope cannot be authorized (fail closed)"
|
||||
)
|
||||
elif workspace_slug.lower() not in {entry.lower() for entry in scope}:
|
||||
reasons.append(
|
||||
f"repository scope denial: workspace repository '{workspace_slug}' "
|
||||
f"is not authorized by profile '{name}' allowed_repositories "
|
||||
f"{sorted(scope)} (fail closed)"
|
||||
)
|
||||
return {
|
||||
"proven": not reasons,
|
||||
"block": bool(reasons),
|
||||
"reasons": reasons,
|
||||
"scope_enforced": True,
|
||||
"workspace_slug": workspace_slug,
|
||||
"allowed_repositories": sorted(scope),
|
||||
}
|
||||
|
||||
|
||||
def assess_repository_override(
|
||||
*,
|
||||
requested_org: str | None,
|
||||
requested_repo: str | None,
|
||||
bound_org: str | None,
|
||||
bound_repo: str | None,
|
||||
) -> dict[str, Any]:
|
||||
"""Caller-supplied org/repo must agree with the immutable binding.
|
||||
|
||||
A mutation request is never allowed to establish, complete, or replace the
|
||||
binding — it may only be checked against it.
|
||||
"""
|
||||
reasons: list[str] = []
|
||||
req_org = (requested_org or "").strip() or None
|
||||
req_repo = (requested_repo or "").strip() or None
|
||||
if req_repo and bound_repo and req_repo.lower() != bound_repo.lower():
|
||||
reasons.append(
|
||||
f"repository override denial: request targets '{req_repo}' but the "
|
||||
f"session is bound to '{bound_repo}' (fail closed)"
|
||||
)
|
||||
if req_org and bound_org and req_org.lower() != bound_org.lower():
|
||||
reasons.append(
|
||||
f"organization override denial: request targets '{req_org}' but the "
|
||||
f"session is bound to '{bound_org}' (fail closed)"
|
||||
)
|
||||
return {"proven": not reasons, "block": bool(reasons), "reasons": reasons}
|
||||
|
||||
|
||||
def filter_profiles_for_remote(
|
||||
config: dict | None,
|
||||
remote: str | None,
|
||||
|
||||
Reference in New Issue
Block a user