Merge branch 'master' into fix/issue-690-review-profile-switch-guard
This commit is contained in:
@@ -0,0 +1,209 @@
|
||||
# The canonical author issue-lock contract (#953)
|
||||
|
||||
Every author issue lock has exactly one shape. Both writers —
|
||||
`gitea_bootstrap_author_issue_worktree` and `gitea_lock_issue` — build it
|
||||
through `author_lock_contract.build_canonical_issue_lock`, and every reader
|
||||
consumes that same shape.
|
||||
|
||||
Before #953 the two writers disagreed. `gitea_lock_issue` wrote the canonical
|
||||
record; bootstrap wrote a thinner one with the claimant at the lock top level,
|
||||
`lease_id: null`, and no `work_lease`, `lock_provenance`, or expiry. Because
|
||||
every reader was written against the canonical shape, a lock that bootstrap
|
||||
reported as successfully created could not be heartbeated, renewed, re-locked,
|
||||
or accepted by `gitea_create_pr`. Each of those gates was individually correct;
|
||||
the defect was that two writers disagreed about what a lock *is*.
|
||||
|
||||
## Required ordering
|
||||
|
||||
**Finalize the lock before writing any implementation bytes.** This ordering is
|
||||
what keeps recovery cheap: while the worktree is still base-equivalent, a lock
|
||||
problem can be fixed by simply calling `gitea_lock_issue` again. Once the branch
|
||||
carries commits, base-equivalence is gone and the ordinary re-lock path is no
|
||||
longer available.
|
||||
|
||||
1. `gitea_whoami` — resolve identity and profile.
|
||||
2. `gitea_resolve_task_capability(task='work_issue')`.
|
||||
3. `gitea_bootstrap_author_issue_worktree` — creates the branch, the registered
|
||||
worktree under `branches/`, and a **canonical** lock. It reads the lock back
|
||||
and verifies it structurally before reporting success; a partial lock fails
|
||||
closed here, with the missing fields named, and never reports
|
||||
`implementation_allowed: true`.
|
||||
4. `gitea_heartbeat_issue_lock` — prove the lock is usable, using the
|
||||
`task_session_id` bootstrap returned.
|
||||
5. Implement, commit, push.
|
||||
6. `gitea_create_pr`.
|
||||
|
||||
If bootstrap returns `success: false` with
|
||||
`reason_code: incomplete_issue_lock_contract`, **do not implement**. Its
|
||||
`exact_next_action` names the executable recovery step. Bootstrap's reported
|
||||
next action always matches the state it actually returned.
|
||||
|
||||
### What that refusal leaves behind
|
||||
|
||||
The AC7 refusal runs `run_compensating_recovery` *before* it reports, so the
|
||||
advice has to describe the post-rollback state rather than the shape of the lock
|
||||
that provoked it. Recommending incomplete-lock recovery for artifacts the
|
||||
rollback already deleted would produce `no_durable_lock` and then
|
||||
`worktree_invalid` — two refusals for a state a plain retry fixes.
|
||||
|
||||
The refusal therefore carries `compensating_recovery` and
|
||||
`post_compensation_state`, and derives `exact_next_action` from what was
|
||||
observed on disk. `cleanup_state` is one of:
|
||||
|
||||
| `cleanup_state` | Meaning | Next action |
|
||||
| --- | --- | --- |
|
||||
| `complete` | lock, branch, and worktree all removed | resolve `missing_fields` and re-run `gitea_bootstrap_author_issue_worktree` |
|
||||
| `partial` | rollback ran; some artifacts survive, by design or because a step errored | scoped to exactly what survives — see below |
|
||||
| `failed` | rollback never completed, so nothing is proven removed | `gitea_inspect_issue_lock_contract` (read-only) before anything else |
|
||||
|
||||
Within `partial`, the surviving set decides the action:
|
||||
|
||||
| Survives | Next action |
|
||||
| --- | --- |
|
||||
| lock + branch + worktree | `gitea_recover_incomplete_bootstrap_lock` for that exact issue, branch, and worktree |
|
||||
| branch + worktree (lock released) | `gitea_lock_issue` — no implementation bytes were written, so the worktree is still base-equivalent |
|
||||
| lock only (worktree removed) | `gitea_inspect_issue_lock_contract`; the surviving lock must be released by its recorded owner before bootstrap is retried |
|
||||
| branch only | `gitea_inspect_issue_lock_contract`, then re-run bootstrap, which adopts the existing branch |
|
||||
|
||||
`failed_rollback_steps` names any rollback step that errored, and the returned
|
||||
action says so rather than presenting the surviving state as intentional.
|
||||
|
||||
> The lock half of that rollback was dead code until #953 review 632 F2:
|
||||
> `run_compensating_recovery` called `issue_lock_store.release_session_lock`,
|
||||
> which did not exist, inside a bare `except Exception: pass`. Every rollback
|
||||
> removed the branch and worktree and silently left the lock — the exact
|
||||
> uninspectable, unrecoverable state this issue exists to eliminate. The
|
||||
> function now exists, releases only a lock whose recorded `owner_session`
|
||||
> matches, and its failures are recorded rather than swallowed.
|
||||
|
||||
## The contract
|
||||
|
||||
A canonical lock carries every field in
|
||||
`author_lock_contract.REQUIRED_LOCK_FIELDS`:
|
||||
|
||||
| Field | Meaning |
|
||||
| --- | --- |
|
||||
| `remote`, `org`, `repo`, `issue_number` | repository and issue identity |
|
||||
| `branch_name`, `worktree_path` | the binding this claim owns |
|
||||
| `work_lease` | the canonical lease block, below |
|
||||
| `lock_provenance` | sanctioned source, minted server-side |
|
||||
| `lock_generation` | monotonic; every write advances it |
|
||||
|
||||
`work_lease` carries every field in
|
||||
`author_lock_contract.REQUIRED_WORK_LEASE_FIELDS`, notably:
|
||||
|
||||
| Field | Meaning |
|
||||
| --- | --- |
|
||||
| `claimant.{username,profile}` | **canonical** claimant placement |
|
||||
| `expires_at` | sliding TTL from `lease_policy` |
|
||||
| `last_heartbeat_at`, `heartbeat_count` | liveness evidence |
|
||||
| `task_session_id` | the ownership fencing token — never null |
|
||||
| `lifecycle_version` | `heartbeat-v1`; its absence is what makes a lock legacy |
|
||||
|
||||
### Claimant placement and legacy compatibility
|
||||
|
||||
`work_lease.claimant` is canonical. A top-level `claimant` is the legacy
|
||||
placement written by pre-#953 bootstrap and is still **read** — through the one
|
||||
shared reader, `issue_lock_store.lock_claimant` — so an existing lock is not
|
||||
refused for "not recording a claimant" when it plainly records one.
|
||||
|
||||
Tolerating the placement is not a widening. Every caller still compares the
|
||||
values against server-resolved identity and profile, so a legacy placement
|
||||
grants nothing the canonical placement would not. When both are present, the
|
||||
`work_lease` copy wins: after an upgrade, a stale top-level copy must never
|
||||
decide ownership.
|
||||
|
||||
### Expiration is explicit
|
||||
|
||||
A lock with no recorded expiry is **not** "not yet expired". `is_lease_expired`
|
||||
returns `False` for it, which used to make such a lock permanently non-expiring
|
||||
*and* permanently ineligible for #760 exact-owner renewal, which only ever
|
||||
assesses an expired lease. `author_lock_contract.expiration_state` names the
|
||||
real fact: `recorded`, `missing`, or `unparseable`. A `missing` expiry makes the
|
||||
lock eligible for the recovery path below rather than stranding it.
|
||||
|
||||
## Recovering an existing incomplete bootstrap lock
|
||||
|
||||
For locks already written by the old bootstrap — including those whose branches
|
||||
already carry legitimate committed and pushed work — use:
|
||||
|
||||
```text
|
||||
gitea_inspect_issue_lock_contract(issue_number, branch_name, worktree_path, remote=...)
|
||||
gitea_recover_incomplete_bootstrap_lock(issue_number, branch_name, worktree_path, expected_head, remote=...)
|
||||
```
|
||||
|
||||
`gitea_inspect_issue_lock_contract` is strictly read-only: it performs no lock,
|
||||
lease, branch, worktree, issue, or pull-request mutation. Use it first to see
|
||||
which fields are missing and what the recommended action is; pass `dry_run=True`
|
||||
to the recovery tool to preview the decision without writing.
|
||||
|
||||
`gitea_recover_incomplete_bootstrap_lock` upgrades that one lock to the
|
||||
canonical contract. Before writing anything it verifies:
|
||||
|
||||
* repository (`remote`, `org`, `repo`) and issue number
|
||||
* claimant username **and** profile against the server-resolved values — a
|
||||
matching username alone is never accepted
|
||||
* branch, worktree path, worktree existence, and worktree registration
|
||||
* the worktree is on the recorded branch
|
||||
* the observed head equals the caller's `expected_head`
|
||||
* the existing lock's generation and provenance state
|
||||
* the absence of healthy foreign ownership
|
||||
|
||||
What it deliberately does **not** do:
|
||||
|
||||
* it never moves, resets, or rewinds the branch, and never requires
|
||||
base-equivalence — preserving the committed work is the entire point;
|
||||
* it never pushes and never creates a pull request;
|
||||
* it touches only the single lock file for that exact remote/org/repo/issue;
|
||||
* it accepts no caller-supplied provenance and no caller-supplied authorization
|
||||
flag — both are minted server-side.
|
||||
|
||||
A recovered lock records a `bootstrap_lock_recovery` block holding both sides of
|
||||
the transition — prior contract, prior missing fields, prior generation, prior
|
||||
owning session, the replacement `task_session_id`, and the preserved head — so a
|
||||
recovered claim never reads as an original one.
|
||||
|
||||
### Gates, in order
|
||||
|
||||
`gitea_recover_incomplete_bootstrap_lock` is an author-only durable-lock
|
||||
mutation and carries the same three gates as every comparable author operation,
|
||||
in this order:
|
||||
|
||||
1. `role_session_router.check_author_mutation_after_reviewer_stop` — no author
|
||||
fallback after a reviewer `wrong_role_stop`.
|
||||
2. `_namespace_mutation_block(task, remote=remote, author_role_exclusive=True)` —
|
||||
the namespace wall. It refuses a reviewer-bound session and, because this
|
||||
task's required permission is `gitea.issue.comment` (which merger,
|
||||
controller, and reconciler profiles also hold), additionally requires the
|
||||
active profile's derived role kind to be exactly `author`. A refusal carries
|
||||
`namespace_block: true` and emits a `BLOCKED` audit record naming the
|
||||
namespace and profile.
|
||||
3. `_profile_permission_block` — operation, provenance, and session-context
|
||||
gates.
|
||||
|
||||
Exact-owner claimant matching inside `assess_bootstrap_lock_recovery` runs
|
||||
*after* all three. It is a further layer, never a substitute for them: on its
|
||||
own it refuses one step too late and leaves the audit trail silent about the
|
||||
attempt.
|
||||
|
||||
### Refusals
|
||||
|
||||
| `refusal_code` | Meaning |
|
||||
| --- | --- |
|
||||
| `no_durable_lock` | nothing to recover |
|
||||
| `already_canonical` | lock is fine; rewriting would invalidate a live heartbeat token |
|
||||
| `foreign_claimant` | recorded claimant is not the active identity/profile pair |
|
||||
| `healthy_foreign_lock` | a live foreign-owned lock; takeover is not a recovery path |
|
||||
| `identity_unresolved` | identity or profile could not be resolved |
|
||||
| `binding_mismatch` | repository, issue, branch, or worktree does not match |
|
||||
| `worktree_invalid` | worktree missing, unregistered, or on another branch |
|
||||
| `head_mismatch` | the worktree moved under the caller |
|
||||
|
||||
## The #447 create-PR provenance guard is unchanged
|
||||
|
||||
`issue_lock_provenance.assess_lock_file_for_create_pr` still requires both a
|
||||
sanctioned `lock_provenance` and a `work_lease`, and the sanctioned source set
|
||||
was **not** widened. Bootstrap writes through
|
||||
`issue_lock_provenance.SOURCE_LOCK_ISSUE` — the lock it produces *is* a
|
||||
canonical lock, not a second dialect with its own exemption. Bootstrap now
|
||||
satisfies the guard rather than the guard being relaxed to admit bootstrap.
|
||||
@@ -0,0 +1,70 @@
|
||||
# MCP Config Drift Diagnostic & Sanctioned Repair Runbook (#672)
|
||||
|
||||
This document describes the diagnostic framework for detecting configuration drift between the active IDE MCP configuration (`~/.gemini/antigravity-ide/mcp_config.json`) and the offline/global canonical configuration (`~/.gemini/config/mcp_config.json`), and establishes the **sanctioned repair runbook**.
|
||||
|
||||
## Background & Problem Statement
|
||||
|
||||
Offline tools like `test_mcp_conn.py` test the global configuration (`~/.gemini/config/mcp_config.json`) via `subprocess.Popen`. However, the active IDE/client namespace uses `~/.gemini/antigravity-ide/mcp_config.json`. When required Gitea role servers (`gitea-author`, `gitea-reviewer`, `gitea-merger`, `gitea-reconciler`, `gitea-controller`, `gitea-tools`) are missing or carry mismatched profile environments in the active IDE config:
|
||||
|
||||
1. Offline tests pass (`test_mcp_conn.py` green).
|
||||
2. The IDE client returns `EOF` / `transport closed` when attempting role-scoped mutations.
|
||||
3. Operators misdiagnose missing server definitions as stale runtimes, leading to forbidden `pkill` attempts (#630) or `mtime` hacks (#655).
|
||||
|
||||
## Diagnostic Tool: `mcp_config_drift.py`
|
||||
|
||||
Run the diagnostic tool directly to compare configurations:
|
||||
|
||||
```bash
|
||||
python3 mcp_config_drift.py --json
|
||||
```
|
||||
|
||||
Or specify custom config locations:
|
||||
|
||||
```bash
|
||||
python3 mcp_config_drift.py \
|
||||
--active-config ~/.gemini/antigravity-ide/mcp_config.json \
|
||||
--global-config ~/.gemini/config/mcp_config.json
|
||||
```
|
||||
|
||||
### Key Diagnostic Outputs
|
||||
|
||||
- `in_sync`: Boolean indicating if all required Gitea role servers exist in the active IDE config with matching profile declarations.
|
||||
- `missing_role_servers`: List of role servers present in global config but missing from active IDE config.
|
||||
- `profile_mismatches`: List of profile environment mismatches per server.
|
||||
- `reasons`: Explicit, human-readable list of drift causes.
|
||||
|
||||
All returned payloads automatically redact secret tokens, DSNs, Authorization headers, and private keys.
|
||||
|
||||
---
|
||||
|
||||
## Sanctioned Repair Path (Step-by-Step)
|
||||
|
||||
When `mcp_config_drift.py` reports drift (`in_sync: false`), execute the following **sanctioned repair steps**:
|
||||
|
||||
1. **Backup Active IDE Config:**
|
||||
```bash
|
||||
cp ~/.gemini/antigravity-ide/mcp_config.json ~/.gemini/antigravity-ide/mcp_config.json.bak
|
||||
```
|
||||
2. **Patch Active IDE Config:**
|
||||
Copy the missing Gitea role server JSON blocks (`gitea-author`, `gitea-reviewer`, etc.) from `~/.gemini/config/mcp_config.json` into `~/.gemini/antigravity-ide/mcp_config.json`.
|
||||
3. **Reconnect via IDE/Client:**
|
||||
Use the IDE / client UI reconnection control (or restart the IDE client app).
|
||||
4. **Verify Active Namespace Health:**
|
||||
Invoke `gitea_whoami` (and optional `gitea_resolve_task_capability`) through the active IDE client on each required role namespace.
|
||||
|
||||
---
|
||||
|
||||
## FORBIDDEN Repair Actions (#630 / #655)
|
||||
|
||||
The following actions are **strictly forbidden** for config drift repair:
|
||||
|
||||
- ❌ **`pkill` or manual daemon process kill commands:** Process kills cause contamination and break active session leases.
|
||||
- ❌ **`mtime` touch edits:** Artificial mtime modifications mask stale runtimes without updating configuration.
|
||||
- ❌ **Source code edits:** Mutating python tool logic to bypass missing server entries.
|
||||
- ❌ **Session-state edits:** Direct database or lock-file state mutation.
|
||||
|
||||
---
|
||||
|
||||
## Final Report Guidelines
|
||||
|
||||
A workflow final report **must not** rely on offline `test_mcp_conn.py` output alone. Final reports must include active-config evidence from live `gitea_whoami` calls on the active IDE namespaces.
|
||||
@@ -47,18 +47,33 @@ Do the steps in order. Stop as soon as a live **client-namespace** call succeeds
|
||||
- Only the Gitea namespace fails → single-namespace transport close. Continue.
|
||||
- Every server fails → restart the whole MCP client, not just one namespace.
|
||||
|
||||
2. **Reconnect the namespace through the client, not the shell.** Use the IDE /
|
||||
client MCP-reconnect action for that server entry (in Claude Code:
|
||||
`/mcp` → reconnect the affected `gitea-*` server). Reconnecting forces the
|
||||
client to spawn a fresh subprocess and re-open the pipe. This clears the
|
||||
closed-client state that a bare `kill`/respawn from a terminal does **not**.
|
||||
2. **Request the sanctioned reconnect surface (#678), then reconnect through
|
||||
the client — not the shell.** From a still-reachable Gitea MCP namespace
|
||||
(or after host auto-reconnect), call:
|
||||
|
||||
```text
|
||||
gitea_request_mcp_reconnect(
|
||||
namespace="gitea-author", # or gitea-reviewer / gitea-merger / …
|
||||
reason="transport_eof",
|
||||
client="codex", # or claude_code / generic
|
||||
)
|
||||
```
|
||||
|
||||
The tool is **report-only**: it never restarts a process. It returns
|
||||
namespace, profile, pid/session, startup SHA, current master SHA, boundary
|
||||
status, and a **typed blocker** with exact operator UI steps for Codex
|
||||
(Reload Developer Tools / per-server reconnect) or Claude Code (`/mcp`).
|
||||
Then perform the host reconnect those steps describe so the client spawns a
|
||||
fresh subprocess and re-opens the pipe. That clears the closed-client state
|
||||
that a bare `kill`/respawn from a terminal does **not**.
|
||||
|
||||
3. **Do not "fix" it by importing the server or poking the process.** Reaching
|
||||
for `python -c 'import gitea_mcp_server ...'`, raw JSON-RPC from a shell,
|
||||
killing PIDs to force a respawn, or touching MCP config mtimes does **not**
|
||||
restore the *client's* view of the namespace and violates the daemon-import
|
||||
guard (#558, `docs/mcp-daemon-import-guard.md`). The only sanctioned repair
|
||||
is a **client reconnect / relaunch**.
|
||||
is a **client reconnect / relaunch** (or the typed operator path returned by
|
||||
`gitea_request_mcp_reconnect`).
|
||||
|
||||
4. **Verify through the same path the workflow will use.** After reconnect, call
|
||||
the specific tool the blocked workflow needs — not just any tool — through
|
||||
@@ -153,7 +168,19 @@ not a tool argument: a session must never be able to authorize itself.
|
||||
## Related
|
||||
|
||||
- #630 — manual daemon killing as contaminated recovery (this contrast, enforced).
|
||||
- #657 — restart-path inventory and daemon classification.
|
||||
- #686 — manual server launch detection & fail-closed provenance gate.
|
||||
- #531 / #544 — stale-runtime detection (`ps`-based); sibling failure mode.
|
||||
- #558 / `docs/mcp-daemon-import-guard.md` — why shell imports are not a repair.
|
||||
- `docs/mcp-client-registration.md` — per-server registration contract.
|
||||
- `docs/mcp-namespace-health.md` — probe sources and mutation enforcement.
|
||||
|
||||
## Sanctioned reconnect vs forbidden manual launch (#686)
|
||||
|
||||
In addition to manual process killing (#630), manually launching a duplicate role server from an ad hoc shell (`python3 mcp_server.py`) is forbidden and fail-closed:
|
||||
|
||||
- **Why manual launches are unsupported:** A terminal-launched `mcp_server.py` holds its own stdio transport; it can never bind to the IDE client's stdio pipes. It cannot restore a dropped IDE namespace, and a manual duplicate process masks stale client-managed runtimes for that profile, defeating stale-runtime gates.
|
||||
- **Sanctioned path:** Supported recovery is IDE/client-managed reconnect only (`/mcp reconnect`, IDE restart, or sanctioned reconnect exposure).
|
||||
- **Fail-closed enforcement (#686):** Mutating tools on a server lacking client-managed launch provenance (`GITEA_CLIENT_MANAGED=1`) refuse execution fail-closed with typed blocker `unsupported_manual_launch` and an exact next action. Unsupported `GITEA_*` env overrides (e.g. `GITEA_DUMMY`) are surfaced in diagnostics rather than silently ignored.
|
||||
- **Inventory & staleness:** Staleness diagnostics ignore non-client-managed duplicates when evaluating runtime freshness and inventory duplicate processes per profile (#657, #686).
|
||||
|
||||
|
||||
@@ -45,10 +45,11 @@ and *fails closed*.
|
||||
| `legacy_auto_restart_helper` | removed | A helper (`_trigger_mcp_auto_restart`) that actively restarted the server from the read-only resolver path. | Removed in #685; kept absent by `assert_auto_restart_helper_absent()`. | #685, #657 |
|
||||
| `config_touch_reload` | removed | Touching (utime) the MCP client config to make the host reload the server. | Removed from the resolver in #685: stale detection is report-only, never mutating config, spawning threads, or calling `os._exit`. | #685, #657 |
|
||||
| `master_advance_auto_restart` | guarded_fail_closed | On-disk master advancing past the running code. | `master_parity_gate` captures startup parity and blocks mutations while stale, emitting restart guidance; the process never self-restarts. | #420, #591, #657 |
|
||||
| `stale_runtime_resolver_reconnect` | guarded_fail_closed | The capability resolver detecting a stale serving process. | Report-only (#685): returns `restart_required`/`stop_required` and an exact reconnect action; no restart, thread, config touch, or `os._exit`. | #685, #657 |
|
||||
| `stale_runtime_resolver_reconnect` | guarded_fail_closed | The capability resolver detecting a stale serving process. | Report-only (#685): returns `restart_required`/`stop_required` and an exact reconnect action; no restart, thread, config touch, or `os._exit`. | #685, #657, #678 |
|
||||
| `codex_client_reconnect_request` | guarded_fail_closed | `gitea_request_mcp_reconnect` report-only tool for Codex/LLM sessions. | Report-only (#678): returns namespace/profile/pid/startup SHA/master SHA/boundary status and a typed operator blocker with exact client UI steps; never restarts or kills. | #678, #630, #685, #657 |
|
||||
| `manual_daemon_kill` | forbidden | Shell kills of the daemon: `pkill -f mcp_server.py`, `killall`, broad `pkill -f python` sweeps, or `kill <pid>` of a daemon pid. | Forbidden (#630): `runtime_recovery_guard` classifies these as contamination and `gitea_record_daemon_process_kill_attempt` writes a durable marker that fails later mutations closed. Operator maintenance authorization is read only from the environment. | #630, #657 |
|
||||
| `conflict_marker_infra_stop` | guarded_fail_closed | The daemon entrypoint scans for unresolved merge-conflict markers at startup and stops (`sys.exit(1)`). | Fail-closed startup stop, not a restart: the process exits and waits for the operator to resolve conflicts and relaunch; never loops. | #657 |
|
||||
| `ide_client_reconnect` | host_residual | A manual `/mcp reconnect` (or equivalent host action) that recreates the MCP client connection. | Outside this process's control; the sanctioned recovery the gates point operators toward. No in-process code initiates it. | #584, #656, #657 |
|
||||
| `ide_client_reconnect` | host_residual | A manual `/mcp reconnect` (or equivalent host action) that recreates the MCP client connection. Agents obtain exact UI steps via `gitea_request_mcp_reconnect` (#678). | Outside this process's control; the sanctioned recovery the gates point operators toward. No in-process code initiates it. | #584, #656, #657, #678 |
|
||||
| `profile_switch_runtime` | sanctioned_narrow_recovery | Switching the active execution profile at runtime (dynamic-profile mode). | In-process and restart-free: `runtime_switching_supported` is true, so a switch rebinds capability without recreating the process. | #656, #657 |
|
||||
|
||||
## Guards enforced in CI
|
||||
|
||||
@@ -103,6 +103,7 @@ that gates each call, not which tools exist.
|
||||
- `gitea_get_shell_health`
|
||||
- `gitea_heartbeat_issue_lock`
|
||||
- `gitea_heartbeat_reviewer_pr_lease`
|
||||
- `gitea_inspect_issue_lock_contract`
|
||||
- `gitea_inspect_workflow_lease`
|
||||
- `gitea_issue_irrecoverable_provenance_authorization`
|
||||
- `gitea_list_dependency_edges`
|
||||
@@ -134,9 +135,11 @@ that gates each call, not which tools exist.
|
||||
- `gitea_record_pre_review_command`
|
||||
- `gitea_record_shell_spawn_outcome`
|
||||
- `gitea_record_stable_branch_push_attempt`
|
||||
- `gitea_recover_incomplete_bootstrap_lock`
|
||||
- `gitea_release_merger_pr_lease`
|
||||
- `gitea_release_reviewer_pr_lease`
|
||||
- `gitea_release_workflow_lease`
|
||||
- `gitea_request_mcp_reconnect`
|
||||
- `gitea_request_mcp_restart`
|
||||
- `gitea_resolve_task_capability`
|
||||
- `gitea_resume_review_draft`
|
||||
|
||||
@@ -0,0 +1,35 @@
|
||||
# Web Console: Sentry/GlitchTip Observability & Incident Bridge Console (#649)
|
||||
|
||||
This document describes the Phase 4 observability console surface integrated into the MCP Control Plane Web Console (`webui/`), backed by the #612 incident bridge and the #613 control-plane DB substrate.
|
||||
|
||||
## Architectural Authority Model (ADR Alignment)
|
||||
|
||||
Per the Web Console Architecture ADR (`docs/architecture/webui-control-plane-console-architecture-adr.md`):
|
||||
|
||||
| Layer | Responsibility | Authority |
|
||||
|---|---|---|
|
||||
| **Gitea** | Durable work record | Issues, PRs, comments, reviews, labels, merges |
|
||||
| **Control-plane DB** | Live coordination & linkage | `incident_links` table, session leases, allocations |
|
||||
| **Sentry / GlitchTip** | Observability input | Unresolved incidents, error events, stack traces |
|
||||
| **Incident Bridge (#612)** | Reconciliation engine | Reconciles provider observations into Gitea issues |
|
||||
| **Web Console (`webui/`)** | Read-only projection & gated actions | Projects connection health & correlation links; gates writes |
|
||||
|
||||
> **Key Rule:** Raw monitoring incidents are **never** assignable control-plane `work_items`. They remain observation input only.
|
||||
|
||||
## Redaction Boundary Invariants
|
||||
|
||||
1. **No secrets in returns or rendering:** Auth tokens (`SENTRY_AUTH_TOKEN`, `GLITCHTIP_AUTH_TOKEN`), DSNs, `Authorization` headers, and sensitive local file paths are passed through `webui.console_redaction` before leaving the server.
|
||||
2. **Safe projection:** Connection objects report `credentials_present: true/false` rather than exposing raw keys or headers.
|
||||
|
||||
## Console Endpoints
|
||||
|
||||
- **HTML Surface:** `GET /observability` — Renders provider connection cards, error correlation tables, and gated reconcile controls.
|
||||
- **Versioned API:** `GET /api/v1/observability` — Returns structured JSON snapshot with `schema_version`, `providers`, `links`, and `metrics`.
|
||||
- **Legacy Compatibility Alias:** `GET /api/observability` — Read-only compatibility alias for Phase 4.
|
||||
|
||||
## Gated Actions
|
||||
|
||||
- `observability_reconcile_incident` (`gitea_observability_reconcile_incident`): Triggers or previews dry-run issue reconciliation for a provider incident.
|
||||
- `observability_link_issue` (`gitea_observability_link_issue`): Links a provider incident to an existing Gitea tracking issue.
|
||||
|
||||
Both actions require `operator` role and gate through `task_capability_map`. Execution fails closed in read-only MVP mode.
|
||||
@@ -0,0 +1,230 @@
|
||||
# Remote-MCP coupling inventory
|
||||
|
||||
Every place the Gitea MCP server depends on being a local, client-spawned, stdio-attached
|
||||
process on the operator's machine.
|
||||
|
||||
- **Issue:** #930 (Remote-MCP 01), child 1 of epic #929.
|
||||
- **Generated against commit:** `7bf4f1258451823a55b36d2157e74f8457165088` (`master`).
|
||||
- **Anchors:** every `file:line` below resolves at the commit above and at the commit that
|
||||
adds this document. This change adds one new file and edits no existing file, so no
|
||||
existing line number shifts between the two.
|
||||
- **Scope:** documentation only. No server behavior changes in this child.
|
||||
|
||||
## How to read an entry
|
||||
|
||||
| Field | Meaning |
|
||||
| ----- | ------- |
|
||||
| **Anchor** | `file:line` at the commit under review. |
|
||||
| **Assumes today** | What the code takes for granted while running as a local stdio process. |
|
||||
| **Observes remotely** | What the same code would actually see on a shared remote host. |
|
||||
| **Class** | One of: *portable as written*, *needs a seam*, *needs a replacement*, *cannot be remote*. |
|
||||
| **Owner** | Exactly one epic child (#931–#939) responsible for the fix. |
|
||||
|
||||
Classification meanings:
|
||||
|
||||
- **portable as written** — the code is already transport-, host-, and principal-neutral; it
|
||||
moves unchanged once its inputs are supplied by a remote-aware caller.
|
||||
- **needs a seam** — the logic is correct but is wired to a hard-coded local source. It needs
|
||||
an injection point, not new semantics.
|
||||
- **needs a replacement** — the semantics themselves are local-only. A remote deployment
|
||||
needs a differently-defined mechanism, not the same mechanism relocated.
|
||||
- **cannot be remote** — the operation is inherently about the operator's own machine
|
||||
(its process table, its keychain, its checkout). It must either stay local behind an
|
||||
explicit boundary or be deleted from the remote surface.
|
||||
|
||||
---
|
||||
|
||||
## 1. Transport bind
|
||||
|
||||
The transport is bound literally, once, at process start, and the bound value is the root of
|
||||
the mutation-authorization chain.
|
||||
|
||||
| ID | Anchor | Assumes today | Observes remotely | Class | Owner |
|
||||
| -- | ------ | ------------- | ----------------- | ----- | ----- |
|
||||
| T1 | `gitea_mcp_server.py:23750` | The single production bind call passes the literal `transport="stdio"` immediately before the server loop. | The literal is wrong for any non-stdio deployment; there is no parameter to change it. | needs a seam | #931 |
|
||||
| T2 | `mcp_daemon_guard.py:45` | `_PRODUCTION_TRANSPORTS = frozenset({"stdio"})` is the closed allowlist of production transports. | A remote transport name is rejected by the allowlist before any other check runs. | needs a seam | #931 |
|
||||
| T3 | `mcp_daemon_guard.py:174` | `bind_native_mcp_transport` raises `UnsanctionedRuntimeError` for any transport outside `_PRODUCTION_TRANSPORTS` (raise at `mcp_daemon_guard.py:187`). | The remote server fails to start rather than degrading; the failure is correct, but the allowlist is the only thing that must change. | needs a seam | #931 |
|
||||
| T4 | `mcp_daemon_guard.py:328` | `is_native_mcp_transport()` asserts a process-local runtime record whose `pid` matches `os.getpid()` and whose phase is `transport_bound`. The predicate itself names no transport. | Unchanged semantics: one server process that bound one transport. It stays true on a remote host. | portable as written | #931 |
|
||||
| T5 | `mcp_daemon_guard.py:349` | `is_production_native_mcp_transport()` adds only a `mode == production` check on top of T4. | Unchanged. | portable as written | #931 |
|
||||
| T6 | `irrecoverable_provenance.py:497` | `assess_transport_for_auth_mint()` requires production native transport before minting non-forgeable recovery authorization (#709 F1). | The gate is transport-agnostic in form, but its guarantee — "an ordinary Python process cannot reach this" — is currently underwritten by the stdio bind. Under a remote transport the guarantee must be re-derived from the authenticated session, not from the bind. | needs a seam | #931 |
|
||||
| T7 | `gitea_mcp_server.py:8375` | Consumer: refuses to proceed unless `assess_transport_for_auth_mint()` allows. | Unchanged given a corrected T6. | portable as written | #931 |
|
||||
| T8 | `gitea_mcp_server.py:8624` | Second consumer of the same gate on the confirmation path. | Unchanged given a corrected T6. | portable as written | #931 |
|
||||
| T9 | `mcp_server.py:4` | Module docstring asserts "Runs over stdio." as a property of the server. | The stated contract becomes false on the remote deployment and is load-bearing documentation for operators. | needs a replacement | #931 |
|
||||
|
||||
## 2. Launch provenance
|
||||
|
||||
Mutations fail closed unless the process can prove a client launched it with real stdio pipes
|
||||
and `GITEA_CLIENT_MANAGED` provenance. Every proof in this section is a statement about the
|
||||
local operating system.
|
||||
|
||||
| ID | Anchor | Assumes today | Observes remotely | Class | Owner |
|
||||
| -- | ------ | ------------- | ----------------- | ----- | ----- |
|
||||
| P1 | `gitea_mcp_server.py:14588` | `_is_client_managed_process()` derives provenance from `GITEA_CLIENT_MANAGED` / `GITEA_MCP_CLIENT_MANAGED` / `GITEA_SERVER_PROVENANCE` / `GITEA_FORCE_CLIENT_MANAGED` on this process's own environment. | A long-lived remote process has one environment for all callers, so a per-process env var can no longer say anything about the caller that issued a request. | needs a replacement | #934 |
|
||||
| P2 | `gitea_mcp_server.py:14606` | Falls back to `sys.stdin.isatty()`: an active TTY on stdin means a human launched it from a terminal, so refuse. | A remote server has no meaningful stdin. The signal is absent, not merely different. | cannot be remote | #934 |
|
||||
| P3 | `gitea_mcp_server.py:14618` | `_provenance_mutation_block()` emits `blocker_kind: "unsupported_manual_launch"` and a "reconnect the IDE/client-managed MCP namespace" remediation. | The block shape is reusable; its predicate and its remediation text are both stdio-specific. | needs a seam | #934 |
|
||||
| P4 | `gitea_mcp_server.py:20599` | `_check_mcp_runtimes_diagnostics()` shells `ps -o pid,lstart,command -ax` and greps for `mcp_server.py` to find peer role servers. | On a shared host the process table lists unrelated tenants' processes, or none at all under a container. Peer discovery by `ps` has no remote meaning. | cannot be remote | #934 |
|
||||
| P5 | `gitea_mcp_server.py:20702` | More than one process per `GITEA_MCP_PROFILE` in the local process table is reported as a duplicate-launch fault. | A remote endpoint is expected to serve many concurrent sessions per role. "Two processes for one role" becomes the normal case, so the check inverts from a safety net into a false wall. | cannot be remote | #934 |
|
||||
| P6 | `gitea_mcp_server.py:20715` | Processes lacking client-managed provenance are ignored for runtime freshness and reported as manual launches. | Same defect as P5: correctness depends on enumerating local peers. | cannot be remote | #934 |
|
||||
| P7 | `gitea_config.py:1172` | `RECOGNIZED_GITEA_ENV_KEYS` is the allowlist of `GITEA_*` env vars a legitimately launched server may carry; anything else is contamination. | Configuration on a remote host arrives from deployment tooling, not from a client-authored env block. The allowlist keeps working mechanically but stops proving anything about provenance. | needs a replacement | #934 |
|
||||
| P8 | `gitea_mcp_server.py:20683` | The unsupported-env scan applies `RECOGNIZED_GITEA_ENV_KEYS` to *other* processes' environments harvested via `ps eww <pid>`. | Reading another process's environment is unavailable or prohibited across tenants, and is not exposed in this form outside macOS/BSD `ps`. | cannot be remote | #934 |
|
||||
| P9 | `mcp_daemon_guard.py:126` | `mark_sanctioned_daemon()` requires the claiming stack frame's resolved absolute path to be the canonical `mcp_server.py` / `gitea_mcp_server.py` next to the guard module; basename spoofing is rejected. | Entrypoint-path identity still exists on a remote host, but it authenticates the *deployment*, not the *caller*. It must be kept and demoted from "authorizes mutations" to "authorizes the process". | needs a seam | #934 |
|
||||
| P10 | `gitea_config.py:1233` | The client-config generator emits `"GITEA_CLIENT_MANAGED": "1"` into each generated MCP client entry, alongside `GITEA_MCP_CONFIG` / `GITEA_MCP_PROFILE`. | A remote endpoint is addressed by URL and credential, not by a spawn command with an env block. This generator produces the wrong artifact entirely. | needs a replacement | #938 |
|
||||
| P11 | `mcp_namespace_health.py:232` | Namespace health classifies a namespace as `client_managed` or `manual_launch` from the reported env summary. | During dual-run, local and remote namespaces coexist and must both be classifiable; a two-valued local/manual axis cannot express "remote endpoint, authenticated session". | needs a replacement | #939 |
|
||||
| P12 | `gitea_mcp_server.py:18161` | The diagnostics payload reports `server_provenance` as exactly `"client_managed"` or `"manual_launch"`. | This is the field a cutover operator reads to confirm which deployment served a call. It must gain a remote value before dual-run parity can be validated. | needs a replacement | #939 |
|
||||
|
||||
## 3. Role binding
|
||||
|
||||
Role separation is currently enforced by *which process a call reaches*. The process is pinned
|
||||
to one role for its lifetime by an environment variable.
|
||||
|
||||
| ID | Anchor | Assumes today | Observes remotely | Class | Owner |
|
||||
| -- | ------ | ------------- | ----------------- | ----- | ----- |
|
||||
| R1 | `gitea_config.py:54` | `ENV_PROFILE = "GITEA_MCP_PROFILE"` is the single source of the active profile, read from the process environment. | One shared process serves several principals; a process-wide profile cannot answer "who is calling now". This is the root of the coupling. | needs a replacement | #932 |
|
||||
| R2 | `review_workflow_load.py:95` | Reads `GITEA_MCP_PROFILE` directly to decide the reviewer workflow binding. | Reads the deployment's profile, not the caller's, silently granting or denying the wrong role. | needs a replacement | #932 |
|
||||
| R3 | `mcp_discoverability.py:152` | Reads `GITEA_MCP_PROFILE` to describe the namespace to the client. | Correct logic, wrong input source; it needs the request principal injected. | needs a seam | #932 |
|
||||
| R4 | `webui/deployment_boundary.py:115` | Reads `GITEA_MCP_PROFILE` to classify the deployment boundary for the console. | Same as R3. | needs a seam | #932 |
|
||||
| R5 | `gitea_mcp_server.py:21106` | Remediation text instructs the operator to "Relaunch the server with `GITEA_MCP_PROFILE` set to a profile that has the required permission". | Relaunching a shared remote endpoint to change one caller's role is not a valid instruction; it would re-role every other session. | needs a replacement | #932 |
|
||||
| R6 | `native_mcp_preference.py:93` | Detects shell commands that override `GITEA_MCP_PROFILE` away from the session (`native_mcp_preference.py:223`) and flags them as CLI auth divergence. | The divergence check is genuinely useful and survives, but its notion of "the session's profile" must come from the request principal. | needs a seam | #932 |
|
||||
| R7 | `gitea_mcp_server.py:20671` | Recovers a peer server's role by regexing `GITEA_MCP_PROFILE=` out of that process's environment. | Depends on P4/P8 process-table access; role discovery by peer-env scraping has no remote analogue. | cannot be remote | #932 |
|
||||
|
||||
## 4. Credentials
|
||||
|
||||
Every token resolves, directly or indirectly, from one human's macOS keychain.
|
||||
|
||||
| ID | Anchor | Assumes today | Observes remotely | Class | Owner |
|
||||
| -- | ------ | ------------- | ----------------- | ----- | ----- |
|
||||
| C1 | `gitea_config.py:956` | `_keychain_token()` shells `security find-generic-password -s <item> -w`. | `security(1)` is a macOS binary reading the calling user's login keychain. It does not exist on a Linux host and would be the wrong identity even on a shared Mac. | cannot be remote | #933 |
|
||||
| C2 | `gitea_config.py:974` | `resolve_token(profile, keychain_lookup=_keychain_token)` dispatches on `auth.type` of `env` or `keychain`, defaulting the lookup to C1. | The injectable `keychain_lookup` parameter is the existing seam; a remote credential provider plugs in here without changing the dispatch. | needs a seam | #933 |
|
||||
| C3 | `gitea_config.py:1015` | `keychain_auth(item_id)` constructs the `{"type": "keychain", "id": ...}` reference stored in profiles. | The reference type itself encodes "macOS keychain" into persisted config. A remote provider needs a new auth reference type, not a new value of this one. | needs a replacement | #933 |
|
||||
| C4 | `mcp_daemon_guard.py:440` | `assert_keychain_access_allowed()` fails closed for git-credential keychain fill outside a sanctioned daemon, with an operator opt-out env var. | The gate protects a mechanism that will not exist remotely. Its replacement must gate the *credential provider* call, not the keychain call, or the protection silently lapses. | needs a replacement | #933 |
|
||||
| C5 | `sentry_incident_bridge.py:190` | `resolve_token(env)` resolves the Sentry token from an injected env mapping with no keychain path. | Already host-neutral; it is the shape the Gitea credential path should converge on. | portable as written | #933 |
|
||||
| C6 | `gitea_mcp_server.py:18469` | The profile-audit tool calls `gitea_config.resolve_token(p)` for every configured profile to report "credentials present" without networking. | On a remote host this would materialize every principal's credential inside one process — an audit surface that becomes a credential-aggregation risk. | needs a seam | #933 |
|
||||
|
||||
## 5. Runtime freshness
|
||||
|
||||
The mutation gate is defined as "the commit this process started at matches the checkout on
|
||||
this disk, and both match live master". Two of those three terms are local-disk facts.
|
||||
|
||||
| ID | Anchor | Assumes today | Observes remotely | Class | Owner |
|
||||
| -- | ------ | ------------- | ----------------- | ----- | ----- |
|
||||
| F1 | `master_parity_gate.py:168` | `capture_startup_parity(root)` reads git `HEAD` from the server's own root once at startup and returns it as the baseline. | A remote host carries a deployed artifact, not the operator's checkout. Its `HEAD` says nothing about the operator's working tree, which is the thing the gate exists to protect. | cannot be remote | #935 |
|
||||
| F2 | `master_parity_gate.py:255` | `mutation_safe = determinable and in_parity and live_known and not live_stale` — a conjunction of two local-HEAD comparisons and one live-remote comparison. | Two of the three conjuncts lose meaning, so the whole verdict does. A remote deployment needs a redefined, testable freshness predicate rather than this one relocated. | needs a replacement | #935 |
|
||||
| F3 | `master_parity_gate.py:164` | The live-remote head is probed and cached per `(root, remote, branch)`, keyed on the local root. | The live-remote probe is the one conjunct that survives; it needs a key that is not the operator's filesystem path. | needs a seam | #935 |
|
||||
| F4 | `gitea_mcp_server.py:18262` | `gitea_assess_master_parity` publishes `startup_head` / `local_head` / `live_remote_head` / `mutation_safe` as the authoritative mutation-safety verdict. | The tool's contract is consumed by every mutation caller and by the operator; it must keep its shape while its semantics are redefined, or every consumer breaks at once. | needs a replacement | #935 |
|
||||
| F5 | `gitea_mcp_server.py:23054` | Falls back to `_process_boot_head_sha` — the commit this process booted at — when the parity payload has no `startup_head`. | Same defect as F1, in a fallback path that is easy to miss when F1 is fixed. | needs a seam | #935 |
|
||||
| F6 | `gitea_mcp_server.py:20615` | Staleness is also inferred from `os.path.getmtime()` of `gitea_mcp_server.py` under `PROJECT_ROOT` (`gitea_mcp_server.py:20611`), compared against peer process start times. | File mtime on a deployed artifact tracks the deploy, not the operator's edits, and the peer start times it is compared against come from the unavailable process table (P4). | cannot be remote | #935 |
|
||||
|
||||
## 6. Local filesystem
|
||||
|
||||
Author and reviewer tools act directly on the operator's checkout.
|
||||
|
||||
| ID | Anchor | Assumes today | Observes remotely | Class | Owner |
|
||||
| -- | ------ | ------------- | ----------------- | ----- | ----- |
|
||||
| L1 | `gitea_mcp_server.py:10122` | `gitea_bootstrap_author_issue_worktree` creates and binds a git worktree on the server's own disk. | The remote host has no operator checkout to add a worktree to. Executing this remotely would act on the wrong disk while reporting success. | cannot be remote | #936 |
|
||||
| L2 | `gitea_mcp_server.py:190` | `ACTIVE_WORKTREE_ENV = "GITEA_ACTIVE_WORKTREE"` and `AUTHOR_WORKTREE_ENV` (`gitea_mcp_server.py:191`) carry the active workspace as process-wide environment. | Process-wide workspace state cannot represent per-session workspaces on a shared endpoint. | needs a replacement | #936 |
|
||||
| L3 | `gitea_mcp_server.py:9801` | Binding a worktree writes `os.environ["GITEA_AUTHOR_WORKTREE"]` and `os.environ["GITEA_ACTIVE_WORKTREE"]` (`gitea_mcp_server.py:9802`), mutating global process state. | One session's bind would silently retarget every other concurrent session in the same process. This is a correctness bug the moment concurrency is real. | needs a replacement | #936 |
|
||||
| L4 | `reviewer_inventory_worktree.py:48` | `_BRANCHES_WORKTREE_RE = re.compile(r"\bbranches/", re.I)` requires review worktree paths to sit under `branches/`. | A path convention on the operator's machine, asserted as a validation rule. It needs to become a property of a declared workspace, not a substring test. | needs a seam | #936 |
|
||||
| L5 | `stable_control_runtime.py:54` | `DEV_WORKTREE_SEGMENT = "branches"` classifies a process root as a development worktree by path segment. | Same class of assumption as L4, on the runtime-classification side. | needs a seam | #936 |
|
||||
| L6 | `mcp_server.py:42` | `check_conflict_markers()` runs at import and `os.walk`s the install directory for unresolved conflict markers, `sys.exit(1)` on a hit. | On a remote host it scans a deployed artifact, which by construction never has conflict markers — so the guard passes trivially and stops protecting the thing it was written to protect. | needs a replacement | #936 |
|
||||
| L7 | `role_session_router.py:487` | `check_mid_merge()` reports infra-stop from `.git/MERGE_HEAD`, `rebase-merge`, `rebase-apply` and a source conflict scan under the server's project root. | Same inversion as L6: it would report the deployment's git state, not the operator's. | needs a replacement | #936 |
|
||||
| L8 | `author_issue_bootstrap.py:996` | Enumerates worktrees with `git -C <root> worktree list --porcelain`. | Requires a real local clone with real worktrees; there is nothing equivalent to enumerate remotely. | cannot be remote | #936 |
|
||||
| L9 | `mcp_server.py:10` | Redirects `sys.stderr` to the fixed path `/tmp/mcp_server_stderr.log` outside pytest. | A single fixed `/tmp` path is shared by every concurrent server on a host and is not a deployment's logging surface. | needs a replacement | #938 |
|
||||
| L10 | `gitea_mcp_server.py:2314` | `ISSUE_LOCK_FILE = "/tmp/gitea_issue_lock.json"` — the legacy single global lock slot. | One global `/tmp` slot per host cannot represent concurrent remote sessions and is world-visible on a shared machine. | needs a replacement | #937 |
|
||||
| L11 | `issue_lock_provenance.py:14` | `ISSUE_LOCK_FILE = os.environ.get("GITEA_ISSUE_LOCK_FILE", "/tmp/gitea_issue_lock.json")` keeps the same `/tmp` default in the provenance path. | Same as L10; the env override is a local escape hatch, not a remote design. | needs a replacement | #937 |
|
||||
|
||||
## 7. Durable state
|
||||
|
||||
Locks, leases, session state, and the control-plane database live in the operator's home
|
||||
directory and are keyed on local PIDs.
|
||||
|
||||
| ID | Anchor | Assumes today | Observes remotely | Class | Owner |
|
||||
| -- | ------ | ------------- | ----------------- | ----- | ----- |
|
||||
| S1 | `issue_lock_store.py:26` | `DEFAULT_LOCK_DIR = ~/.cache/gitea-tools/issue-locks` — per-issue lock files under one user's home. | A shared endpoint has no single operator home; per-user paths make locks invisible across sessions and hosts. | needs a replacement | #937 |
|
||||
| S2 | `issue_lock_store.py:83` | `session_pointer_path()` names the session pointer file `session-<os.getpid()>.json`. | Many sessions share one PID on a remote server, so the pointer collapses to a single slot and sessions overwrite each other. | cannot be remote | #937 |
|
||||
| S3 | `issue_lock_store.py:98` | `is_process_alive(pid)` decides lock liveness by probing the local process table. | A PID recorded by one host is meaningless on another, and may coincidentally match a live unrelated process. | cannot be remote | #937 |
|
||||
| S4 | `issue_lock_store.py:213` | Lock records stamp `session_pid` and `pid` from `os.getpid()`. | The recorded identity no longer distinguishes sessions; ownership checks silently pass for the wrong caller. | needs a replacement | #937 |
|
||||
| S5 | `mcp_session_state.py:27` | `DEFAULT_STATE_DIR = ~/.cache/gitea-tools/session-state`, mode `0o700`. | Same home-directory coupling as S1, for review decision locks and workflow proofs. | needs a replacement | #937 |
|
||||
| S6 | `mcp_session_state.py:559` | Session bodies stamp `session_pid` and `writer_pid` from `os.getpid()` (`mcp_session_state.py:560`). | Writer attribution collapses across concurrent sessions in one process. | needs a replacement | #937 |
|
||||
| S7 | `control_plane_db.py:47` | `DEFAULT_DB_PATH = ~/.cache/gitea-tools/control-plane/control_plane.sqlite3`. | A per-user SQLite file is not reachable by, or safe for, multiple remote sessions or multiple hosts. | needs a replacement | #937 |
|
||||
| S8 | `control_plane_db.py:386` | `sqlite3.connect(self.db_path, timeout=30)` — single-writer file locking tuned for one local process. | SQLite's write lock does not extend across hosts and degrades sharply under real concurrency; the store needs a concurrency-safe backend. | needs a replacement | #937 |
|
||||
| S9 | `control_plane_db.py:1145` | Lease rows record `owner_pid` defaulting to `os.getpid()` (also `control_plane_db.py:2039`). | PID-keyed lease ownership is unusable across hosts and ambiguous within one shared process. | cannot be remote | #937 |
|
||||
| S10 | `mcp_daemon_guard.py:53` | `_DEFAULT_SESSION_STATE_DIR` is pinned once at transport bind so a later `GITEA_MCP_SESSION_STATE_DIR` change cannot manufacture a second authority domain (#695 AC2). | The single-authority-domain invariant is exactly right and must be preserved; only its backing location needs to move. | needs a seam | #937 |
|
||||
| S11 | `gitea_mcp_server.py:11875` | Reviewer-lease reclaim reads `owner_pid_alive` from the lease freshness record to decide whether an owner is dead. | Consumes S3/S9; a false "owner alive" or "owner dead" here reclaims or refuses a live lease. This is the highest-consequence consumer of PID liveness. | cannot be remote | #937 |
|
||||
|
||||
---
|
||||
|
||||
## Summary
|
||||
|
||||
### Entries per category
|
||||
|
||||
| Category | Entries |
|
||||
| -------- | ------: |
|
||||
| 1. Transport bind | 9 |
|
||||
| 2. Launch provenance | 12 |
|
||||
| 3. Role binding | 7 |
|
||||
| 4. Credentials | 6 |
|
||||
| 5. Runtime freshness | 6 |
|
||||
| 6. Local filesystem | 11 |
|
||||
| 7. Durable state | 11 |
|
||||
| **Total** | **62** |
|
||||
|
||||
No category is empty, so no "this category has no coupling" justification is required.
|
||||
|
||||
### Entries per classification
|
||||
|
||||
| Classification | Entries |
|
||||
| -------------- | ------: |
|
||||
| portable as written | 5 |
|
||||
| needs a seam | 16 |
|
||||
| needs a replacement | 26 |
|
||||
| cannot be remote | 15 |
|
||||
| **Total** | **62** |
|
||||
|
||||
### Category × classification
|
||||
|
||||
| Category | portable | seam | replacement | cannot | Total |
|
||||
| -------- | -------: | ---: | ----------: | -----: | ----: |
|
||||
| 1. Transport bind | 4 | 4 | 1 | 0 | 9 |
|
||||
| 2. Launch provenance | 0 | 2 | 5 | 5 | 12 |
|
||||
| 3. Role binding | 0 | 3 | 3 | 1 | 7 |
|
||||
| 4. Credentials | 1 | 2 | 2 | 1 | 6 |
|
||||
| 5. Runtime freshness | 0 | 2 | 2 | 2 | 6 |
|
||||
| 6. Local filesystem | 0 | 2 | 7 | 2 | 11 |
|
||||
| 7. Durable state | 0 | 1 | 6 | 4 | 11 |
|
||||
| **Total** | **5** | **16** | **26** | **15** | **62** |
|
||||
|
||||
### Entries per epic child
|
||||
|
||||
Every child from 2 through 10 is named by at least one entry, and every entry names exactly
|
||||
one child.
|
||||
|
||||
| Child | Issue | Title | Entries | IDs |
|
||||
| ----: | ----- | ----- | ------: | --- |
|
||||
| 2 | #931 | Transport-neutral bind seam | 9 | T1–T9 |
|
||||
| 3 | #932 | Per-request principal resolution | 7 | R1–R7 |
|
||||
| 4 | #933 | Server-side credential provider | 6 | C1–C6 |
|
||||
| 5 | #934 | Remote-session provenance | 9 | P1–P9 |
|
||||
| 6 | #935 | Redefined master-parity gate | 6 | F1–F6 |
|
||||
| 7 | #936 | Local-filesystem vs remotable tool split | 8 | L1–L8 |
|
||||
| 8 | #937 | Concurrency-safe session, lock, and lease state | 13 | L10, L11, S1–S11 |
|
||||
| 9 | #938 | Authenticated remote MCP endpoint | 2 | P10, L9 |
|
||||
| 10 | #939 | Dual-run cutover and rollback | 2 | P11, P12 |
|
||||
| | | **Total** | **62** | |
|
||||
|
||||
## Notes for downstream children
|
||||
|
||||
- **The three highest-risk entries are P5, F2, and S11.** Each is a guard that does not
|
||||
merely stop working remotely — it inverts. P5 turns concurrency into a reported fault,
|
||||
F2 returns a verdict computed from terms that no longer mean anything, and S11 reclaims
|
||||
or refuses leases on a PID-liveness answer that is wrong rather than unknown. A gate that
|
||||
fails open while still reporting green is worse than one that fails to start.
|
||||
- **T4, T5, T7, T8, and C5 are the portable core.** They show the target shape: predicates
|
||||
over injected inputs, with no reference to the host, the process table, or the operator's
|
||||
disk.
|
||||
- **The keychain seam already exists** at C2 (`resolve_token`'s injectable `keychain_lookup`).
|
||||
#933 should widen that seam rather than introduce a parallel path, and must remember C4 —
|
||||
the guard protecting the old mechanism has to be re-pointed, or the protection lapses
|
||||
silently when the mechanism is replaced.
|
||||
- **`branches/` appears as a validation rule in at least two independent places** (L4, L5).
|
||||
Path-substring conventions tend to have more copies than expected; #936 should re-grep
|
||||
rather than trust this list to be exhaustive for that specific pattern.
|
||||
@@ -0,0 +1,80 @@
|
||||
{
|
||||
"_comment": [
|
||||
"Machine-checkable anchor table for docs/remote-mcp/threat-model.md (#956).",
|
||||
"Every file:line anchor cited in the threat model must appear here, and the",
|
||||
"source line at that anchor must contain the 'expect' substring.",
|
||||
"tests/test_issue_956_threat_model.py enforces both directions, so a refactor",
|
||||
"that shifts a line number fails the suite instead of silently rotting the",
|
||||
"document. #930's inventory had no such guard and its gitea_mcp_server.py",
|
||||
"anchors drifted between 7bf4f125 and aad5c8b4."
|
||||
],
|
||||
"generated_against_commit": "aad5c8b42361d380a8eeb07b94b90815e594c2c5",
|
||||
"anchors": [
|
||||
{"anchor": "gitea_mcp_server.py:24721", "expect": "bind_native_mcp_transport(transport=\"stdio\")"},
|
||||
{"anchor": "mcp_daemon_guard.py:45", "expect": "_PRODUCTION_TRANSPORTS = frozenset({\"stdio\"})"},
|
||||
{"anchor": "mcp_daemon_guard.py:174", "expect": "def bind_native_mcp_transport"},
|
||||
{"anchor": "irrecoverable_provenance.py:497", "expect": "def assess_transport_for_auth_mint"},
|
||||
{"anchor": "gitea_mcp_server.py:9129", "expect": "assess_transport_for_auth_mint()"},
|
||||
{"anchor": "gitea_mcp_server.py:9378", "expect": "assess_transport_for_auth_mint()"},
|
||||
{"anchor": "mcp_server.py:4", "expect": "Runs over stdio."},
|
||||
|
||||
{"anchor": "gitea_mcp_server.py:15412", "expect": "def _is_client_managed_process"},
|
||||
{"anchor": "gitea_mcp_server.py:15442", "expect": "def _provenance_mutation_block"},
|
||||
{"anchor": "gitea_mcp_server.py:15450", "expect": "unsupported_manual_launch"},
|
||||
{"anchor": "gitea_mcp_server.py:19001", "expect": "server_provenance"},
|
||||
{"anchor": "gitea_mcp_server.py:21442", "expect": "def _check_mcp_runtimes_diagnostics"},
|
||||
{"anchor": "gitea_mcp_server.py:21462", "expect": "\"ps\", \"-o\", \"pid,lstart,command\""},
|
||||
{"anchor": "gitea_mcp_server.py:21506", "expect": "\"ps\", \"eww\""},
|
||||
{"anchor": "gitea_config.py:1172", "expect": "RECOGNIZED_GITEA_ENV_KEYS"},
|
||||
{"anchor": "gitea_config.py:1233", "expect": "GITEA_CLIENT_MANAGED"},
|
||||
|
||||
{"anchor": "gitea_config.py:54", "expect": "ENV_PROFILE = \"GITEA_MCP_PROFILE\""},
|
||||
{"anchor": "gitea_config.py:97", "expect": "_REVIEW_MERGE_OPS"},
|
||||
{"anchor": "gitea_config.py:499", "expect": "repository authorization scope"},
|
||||
|
||||
{"anchor": "gitea_config.py:956", "expect": "def _keychain_token"},
|
||||
{"anchor": "gitea_config.py:974", "expect": "def resolve_token"},
|
||||
{"anchor": "gitea_config.py:1015", "expect": "def keychain_auth"},
|
||||
{"anchor": "gitea_config.py:294", "expect": "def _validate_identity_auth"},
|
||||
{"anchor": "mcp_daemon_guard.py:440", "expect": "def assert_keychain_access_allowed"},
|
||||
{"anchor": "gitea_mcp_server.py:19258", "expect": "def gitea_list_profiles"},
|
||||
{"anchor": "gitea_mcp_server.py:19309", "expect": "gitea_config.resolve_token(p)"},
|
||||
{"anchor": "gitea_mcp_server.py:19552", "expect": "def gitea_audit_config"},
|
||||
{"anchor": "gitea_mcp_server.py:19574", "expect": "service_summaries(config)"},
|
||||
|
||||
{"anchor": "gitea_config.py:704", "expect": "def resolve_service"},
|
||||
{"anchor": "gitea_config.py:837", "expect": "def service_summaries"},
|
||||
{"anchor": "gitea_config.py:851", "expect": "_keychain_token(auth.get(\"id\"))"},
|
||||
{"anchor": "gitea_mcp_server.py:17707", "expect": "\"jenkins-mcp\""},
|
||||
{"anchor": "gitea_mcp_server.py:17713", "expect": "external-mcp"},
|
||||
{"anchor": "gitea_mcp_server.py:17734", "expect": "\"glitchtip-mcp\""},
|
||||
{"anchor": "gitea_mcp_server.py:17739", "expect": "external-mcp"},
|
||||
{"anchor": "mcp_discoverability.py:9", "expect": "EXPECTED_JENKINS_TOOLS"},
|
||||
{"anchor": "mcp_discoverability.py:17", "expect": "EXPECTED_GLITCHTIP_TOOLS"},
|
||||
|
||||
{"anchor": "sentry_incident_bridge.py:36", "expect": "SENTRY_AUTH_TOKEN"},
|
||||
{"anchor": "sentry_incident_bridge.py:190", "expect": "def resolve_token"},
|
||||
{"anchor": "sentry_incident_bridge.py:289", "expect": "Authorization"},
|
||||
{"anchor": "sentry_observability.py:55", "expect": "SENTRY_DSN"},
|
||||
|
||||
{"anchor": "master_parity_gate.py:168", "expect": "def capture_startup_parity"},
|
||||
{"anchor": "master_parity_gate.py:255", "expect": "mutation_safe"},
|
||||
{"anchor": "gitea_mcp_server.py:19102", "expect": "def gitea_assess_master_parity"},
|
||||
|
||||
{"anchor": "gitea_mcp_server.py:190", "expect": "ACTIVE_WORKTREE_ENV"},
|
||||
{"anchor": "gitea_mcp_server.py:191", "expect": "AUTHOR_WORKTREE_ENV"},
|
||||
{"anchor": "gitea_mcp_server.py:2348", "expect": "/tmp/gitea_issue_lock.json"},
|
||||
{"anchor": "gitea_mcp_server.py:10894", "expect": "def gitea_bootstrap_author_issue_worktree"},
|
||||
{"anchor": "mcp_server.py:10", "expect": "/tmp/mcp_server_stderr.log"},
|
||||
|
||||
{"anchor": "issue_lock_store.py:26", "expect": "DEFAULT_LOCK_DIR"},
|
||||
{"anchor": "issue_lock_store.py:83", "expect": "def session_pointer_path"},
|
||||
{"anchor": "issue_lock_store.py:98", "expect": "def is_process_alive"},
|
||||
{"anchor": "mcp_session_state.py:27", "expect": "DEFAULT_STATE_DIR"},
|
||||
{"anchor": "control_plane_db.py:47", "expect": "DEFAULT_DB_PATH"},
|
||||
{"anchor": "control_plane_db.py:380", "expect": "mode=0o700"},
|
||||
{"anchor": "control_plane_db.py:386", "expect": "sqlite3.connect"},
|
||||
{"anchor": "control_plane_db.py:1145", "expect": "os.getpid()"},
|
||||
{"anchor": "gitea_mcp_server.py:12801", "expect": "owner_pid_alive"}
|
||||
]
|
||||
}
|
||||
@@ -0,0 +1,403 @@
|
||||
# Remote-MCP threat model, trust boundaries, and service decomposition
|
||||
|
||||
What the adversary is, what each boundary protects, and which services may share a process.
|
||||
|
||||
- **Issue:** #956 (Remote-MCP threat model), child of epic #929, cross-linked to #955.
|
||||
- **Depends on:** #930 (closed) — `docs/remote-mcp/coupling-inventory.md`.
|
||||
- **Blocks:** #932, #933, #934, #938.
|
||||
- **Generated against commit:** `aad5c8b42361d380a8eeb07b94b90815e594c2c5` (`master`).
|
||||
- **Scope:** documentation only. This child changes no server behavior. It adds one
|
||||
document, one anchor fixture, and the test that enforces them.
|
||||
|
||||
## Relationship to #930
|
||||
|
||||
#930 asked *what breaks when the process stops being local*. This document asks *what an
|
||||
attacker gets, and where we stop them*. The two are deliberately different axes: #930
|
||||
classifies each coupling as portable, seam, replacement, or cannot-be-remote; this document
|
||||
classifies each **credential** by blast radius and each **boundary** by what crossing it
|
||||
requires. An entry can be perfectly portable and still be a trust disaster —
|
||||
`gitea_config.py:851` is portable Python that reads a CI secret from inside the Gitea server.
|
||||
|
||||
### Anchors are enforced, not asserted
|
||||
|
||||
Every `file:line` in this document is declared in `docs/remote-mcp/threat-model-anchors.json`
|
||||
with the substring that must appear at that line, and
|
||||
`tests/test_issue_956_threat_model.py` fails if any anchor does not resolve or if the
|
||||
document cites an anchor the fixture does not cover.
|
||||
|
||||
This guard exists because #930 did not have one. Its inventory was generated at
|
||||
`7bf4f125`; by `aad5c8b4` its `gitea_mcp_server.py` anchors had drifted — the transport
|
||||
bind it cited at line 23750 now lives at `gitea_mcp_server.py:24721`, and its
|
||||
client-managed provenance anchor at 14588 now lands in an unrelated function. Nothing
|
||||
failed, because nothing checked. Anchors into a ~24,700-line module rot silently, and a
|
||||
security document that cannot prove its own citations is worse than none, because it is
|
||||
trusted.
|
||||
|
||||
---
|
||||
|
||||
## 1. Assets
|
||||
|
||||
What an adversary wants. Ordered by consequence, not by likelihood.
|
||||
|
||||
| ID | Asset | Why it matters |
|
||||
| -- | ----- | -------------- |
|
||||
| A1 | Merge authority on `Scaled-Tech-Consulting/Gitea-Tools` | This repository *is* the control plane. Code merged here becomes the gate that authorizes every future mutation, so merge authority is self-amplifying: one merge can disable every other control in this document. |
|
||||
| A2 | Write authority on the `mdcps` tenant | A second, unrelated organization reachable from the same configuration. Compromise here is a cross-organization incident, not an internal one. |
|
||||
| A3 | The eight Gitea role credentials | Long-lived bearer tokens. Possession is authority; there is no second factor at the API. |
|
||||
| A4 | Jenkins read access (`mdcps`, enabled) | Build logs routinely carry deployment topology, internal hostnames, and accidentally-echoed secrets. |
|
||||
| A5 | Error-tracking read access (GlitchTip / Sentry) | Event payloads carry stack frames, request context, and production user data. |
|
||||
| A6 | Coordination-state integrity | The locks, leases, and review-decision records that make "exactly one owner" true. Corrupting them needs no Gitea credential and produces duplicate or lost work. |
|
||||
| A7 | The operator's checkout and worktrees | Unmerged code, branch state, and the filesystem the author tools write to. |
|
||||
| A8 | The macOS login keychain | The meta-credential. Everything in A3, A4, and A5 resolves from it. |
|
||||
| A9 | Separation of duty between review and merge | The property that no single actor both approves and lands a change. An *asset*, not a control, because it is what the controls exist to produce. |
|
||||
| A10 | Audit and provenance records | Determine whether an incident is reconstructable. An attacker who can forge provenance makes an intrusion indistinguishable from normal work. |
|
||||
|
||||
## 2. Adversaries
|
||||
|
||||
| ID | Adversary | Capability assumed | Not assumed |
|
||||
| -- | --------- | ------------------ | ----------- |
|
||||
| ADV1 | **Compromised LLM client** | Full control of one MCP client. Issues arbitrary tool calls, in any order, with any arguments, at machine speed. Sees every tool result. | Cannot read the operator's disk except through tools; cannot execute arbitrary local code outside the tool surface. |
|
||||
| ADV2 | **Prompt injection** via repository content | Controls text the model reads and treats as instruction — issue bodies, PR descriptions, review comments, commit messages, file contents. Reaches the model on any read of untrusted content. | Holds no credential and issues no call directly. Its entire power is causing an *authorized* client to act. |
|
||||
| ADV3 | **Malicious tool arguments** | Supplies hostile values to any parameter — paths, branch names, session identifiers, worktree paths, issue numbers — including traversal, injection, and confusion between look-alike identifiers. | Cannot bypass a gate that actually validates its input. |
|
||||
| ADV4 | **Network attacker** | Observes and modifies traffic between client, server, and Gitea. Attempts downgrade, replay, and endpoint impersonation. | Does not hold a valid credential at the start. |
|
||||
| ADV5 | **Curious operator** | Legitimate local access to the workstation: process table, `/tmp`, home directory, keychain prompts. Not malicious, but not authorized for every role either. | Does not defeat the OS keychain's own access control without a prompt. |
|
||||
|
||||
ADV2 is the adversary this architecture most under-models. Every other adversary must first
|
||||
obtain something. Prompt injection obtains nothing: it borrows authority the client already
|
||||
holds and is indistinguishable at the tool boundary from legitimate work. Each boundary
|
||||
below therefore states whether it constrains ADV2 at all — and most do not, because they
|
||||
authenticate the *caller*, not the *intent*.
|
||||
|
||||
## 3. Trust boundaries
|
||||
|
||||
"Crossing requires today" is what the code actually enforces at
|
||||
`aad5c8b42361d380a8eeb07b94b90815e594c2c5`, not what the design intends.
|
||||
|
||||
| ID | Boundary | Protects | Crossing requires today | Crossing must require remotely |
|
||||
| -- | -------- | -------- | ----------------------- | ------------------------------ |
|
||||
| B1 | LLM client ↔ MCP server session | A1, A3, A10 — that a mutating session was established through the sanctioned client path | A literal `stdio` bind (`gitea_mcp_server.py:24721`) inside a closed allowlist (`mcp_daemon_guard.py:45`, `mcp_daemon_guard.py:174`); client-managed provenance (`gitea_mcp_server.py:15412`) or a refusal (`gitea_mcp_server.py:15450`); production transport before recovery-authorization mint (`irrecoverable_provenance.py:497`, consumed at `gitea_mcp_server.py:9129` and `gitea_mcp_server.py:9378`) | An authenticated handshake issuing a server-side session identity bound to a principal, with the transport recorded in provenance. The physical proof (a pipe) must become a cryptographic one. |
|
||||
| B2 | Role ↔ role | A9 — that author, reviewer, merger, and reconciler are distinct authorities | **The process boundary only.** The role is a property of the process, read once from `GITEA_MCP_PROFILE` (`gitea_config.py:54`). A caller gets author permissions by connecting to the author process. Review and merge are the operations singled out for extra care (`gitea_config.py:97`) | A per-request principal, so the role follows from the credential presented and cannot be selected by reaching a different endpoint. |
|
||||
| B3 | MCP server ↔ credential store | A3, A8 — that only sanctioned code turns a profile into a token | `_keychain_token` shelling out to the login keychain (`gitea_config.py:956`), dispatched by `resolve_token` (`gitea_config.py:974`) with the reference type built at `gitea_config.py:1015`, gated by `assert_keychain_access_allowed` (`mcp_daemon_guard.py:440`). Inline secrets are rejected at config load (`gitea_config.py:294`) | A credential provider keyed by the *request* principal, returning only that principal's credential, with the source recorded and the value never returned. |
|
||||
| B4 | MCP server ↔ Gitea | A1, A2 — that only authorized calls reach the forge | A bearer token over TLS. Server-side, nothing distinguishes one role's token from another beyond the account it belongs to | Unchanged at the forge; the endpoint in front of it must refuse unauthenticated and plaintext connections before tool dispatch. |
|
||||
| B5 | MCP server ↔ caller's filesystem | A7 — that a tool acts on the *caller's* disk or refuses | Nothing. The server's disk *is* the caller's disk. Worktree bootstrap writes directly (`gitea_mcp_server.py:10894`); the active workspace is process-global (`gitea_mcp_server.py:190`, `gitea_mcp_server.py:191`) | An explicit per-tool classification, enforced at dispatch, refusing filesystem tools over a transport that cannot reach the caller's disk. A green verdict about the wrong disk is the failure to prevent. |
|
||||
| B6 | MCP server ↔ coordination state | A6, A9 — mutual exclusion | Local files and a local SQLite database, with liveness judged from the local process table (`issue_lock_store.py:98`), keyed on paths under one user's home (`issue_lock_store.py:26`, `mcp_session_state.py:27`, `control_plane_db.py:47`) and on `os.getpid()` (`control_plane_db.py:1145`, `gitea_mcp_server.py:12801`). A legacy global slot still exists at `gitea_mcp_server.py:2348`, and the session-pointer file is named per PID (`issue_lock_store.py:83`) | One authority per ownership question, with liveness from session identity and expiry, and atomic acquire, renew, and release across hosts. |
|
||||
| B7 | Gitea integration ↔ unrelated integrations | A4, A5 — that a Gitea compromise is not a CI and observability compromise | **Nothing.** See §5. The Gitea server reads Jenkins and GlitchTip secrets (`gitea_config.py:851`, reached from `gitea_config.py:837`) and holds the Sentry token (`sentry_incident_bridge.py:190`) | A hard process boundary. This is the boundary #956 exists to create. |
|
||||
| B8 | Tenant ↔ tenant (`prgs` / `mdcps` / `local-lab`) | A2 — that one organization's compromise is not another's | Convention. One configuration declares all three contexts; `resolve_service` fails closed on a *disabled* context (`gitea_config.py:704`) but the credentials of enabled ones remain reachable in-process. A per-profile repository scope exists (`gitea_config.py:499`) | Separate deployments, or at minimum per-tenant credential scopes with no process able to resolve both. |
|
||||
| B9 | Deployed code ↔ merged policy | A1, A10 — that the running server enforces the rules that were actually merged | Comparing this process's startup commit against this disk (`master_parity_gate.py:168`), conjoined into a single verdict (`master_parity_gate.py:255`) published by `gitea_mcp_server.py:19102` | Freshness defined against the deployed build identity, with an explicit fail-closed verdict when undeterminable. |
|
||||
|
||||
### What no boundary constrains
|
||||
|
||||
None of B1–B9 constrains **ADV2**. Every one authenticates a caller or a process; prompt
|
||||
injection supplies neither. An injected instruction that reaches an authorized author
|
||||
session crosses B1, B2, B3, and B5 legitimately, because at each of those boundaries it *is*
|
||||
the author. The only controls that bite ADV2 are those constraining what an authenticated
|
||||
principal may do regardless of what it asks for — the per-role permission split (B2), the
|
||||
repository scope at `gitea_config.py:499`, and separation of duty (A9). Sizing those
|
||||
controls correctly matters more after the migration, not less, because a remote endpoint
|
||||
raises the number of clients that can be injected into.
|
||||
|
||||
## 4. Data flows
|
||||
|
||||
Flows that cross a boundary. `==>` carries a credential; `-->` does not.
|
||||
|
||||
```
|
||||
B1 B4
|
||||
[LLM client] ====================> [MCP server] ========> [Gitea]
|
||||
^ stdio pipe today | ^ (A1,A2)
|
||||
| session identity | |
|
||||
| after migration | |
|
||||
| | | B3
|
||||
untrusted repository content | +======> [macOS login keychain] (A8)
|
||||
read back into the model (ADV2) | resolves A3, A4, A5
|
||||
^ |
|
||||
+----------------------------------+
|
||||
|
|
||||
B5 | B6
|
||||
[operator checkout / worktrees] <--------+-------> [locks · leases · sqlite]
|
||||
(A7) | (A6)
|
||||
|
|
||||
B7 <-- boundary does not exist today
|
||||
|
|
||||
+========================+========================+
|
||||
| | |
|
||||
[Jenkins] (A4) [GlitchTip] (A5) [Sentry] (A5)
|
||||
external MCP server external MCP server in-process bridge
|
||||
```
|
||||
|
||||
Two flows deserve attention because neither is obvious from the code:
|
||||
|
||||
1. **The keychain flow fans out.** B3 is drawn once but resolves credentials for *every*
|
||||
configured profile and service, not only the active one. `gitea_list_profiles`
|
||||
(`gitea_mcp_server.py:19258`) reports each profile's credential status by calling
|
||||
`resolve_token` on it (`gitea_mcp_server.py:19309`), and `gitea_audit_config`
|
||||
(`gitea_mcp_server.py:19552`) reports service credential status through
|
||||
`service_summaries` (`gitea_mcp_server.py:19574`).
|
||||
2. **The return path is a flow too.** Content read from Gitea travels back into the model
|
||||
and is treated as instruction. This is the ADV2 edge, and it is the only edge in the
|
||||
diagram with no authentication on it, because it is not a request.
|
||||
|
||||
## 5. Per-boundary credential inventory
|
||||
|
||||
**14 credentials in total.** Blast radius is stated as what the credential yields *on its
|
||||
own*, assuming every gate not backed by the credential itself has been bypassed — because
|
||||
an attacker holding a token calls the API, not our tools.
|
||||
|
||||
| ID | Credential | Holder | Boundary | Blast radius |
|
||||
| -- | ---------- | ------ | -------- | ------------ |
|
||||
| CR1 | `prgs-author` Gitea token — account `jcwalker3` | macOS keychain; resolved in-process (`gitea_config.py:974`) | B3 → B4 | Create branches, push, commit, open PRs, create/close/comment issues on the control-plane repo. Cannot approve or merge. The one credential whose identity is genuinely distinct. |
|
||||
| CR2 | `prgs-reviewer` Gitea token — account `sysadmin` | macOS keychain | B3 → B4 | Approve and request changes. **Shares one Gitea account with CR3, CR4, CR5.** |
|
||||
| CR3 | `prgs-merger` Gitea token — account `sysadmin` | macOS keychain | B3 → B4 | Merge to `master` — A1 in full. Same account as CR2. |
|
||||
| CR4 | `prgs-reconciler` Gitea token — account `sysadmin` | macOS keychain | B3 → B4 | Close PRs, delete branches, irrecoverable decision-lock recovery. Same account as CR2. |
|
||||
| CR5 | `prgs-controller` Gitea token — account `sysadmin` | macOS keychain | B3 → B4 | Same operation set as CR4. Same account as CR2. |
|
||||
| CR6 | `mdcps-author` Gitea token — account `913443` | macOS keychain | B3 → B4, B8 | Author operations on a second organization. **Shares one account with CR7 and CR8.** |
|
||||
| CR7 | `mdcps-reviewer` Gitea token — account `913443` | macOS keychain | B3 → B4, B8 | Approve and request changes on `mdcps`. Same account as CR6. |
|
||||
| CR8 | `mdcps-merger` Gitea token — account `913443` | macOS keychain | B3 → B4, B8 | Merge on `mdcps` — A2 in full. Same account as CR6. |
|
||||
| CR9 | MDCPS Jenkins read credential | macOS keychain, read from the Gitea server process (`gitea_config.py:851`) | B7 | Read CI jobs, builds, and logs (A4). Enabled today. |
|
||||
| CR10 | MDCPS GlitchTip read credential | macOS keychain, read from the Gitea server process (`gitea_config.py:851`) | B7 | Read error events and their payloads (A5). Enabled today. |
|
||||
| CR11 | `SENTRY_AUTH_TOKEN` | Process environment, read in-process (`sentry_incident_bridge.py:36`, `sentry_incident_bridge.py:190`), sent as a bearer header (`sentry_incident_bridge.py:289`) | B7 | Read and reconcile Sentry issues (A5). Not a keychain credential — an env var, so it is inherited by anything the process spawns. |
|
||||
| CR12 | `SENTRY_DSN` | Process environment (`sentry_observability.py:55`) | B7 | Write events into the observability project. Low read value, real forgery value: an attacker can inject fabricated events into the record (A10). |
|
||||
| CR13 | macOS login keychain access | The operator's login session; gated by `assert_keychain_access_allowed` (`mcp_daemon_guard.py:440`) | B3, ADV5 | **Every other credential in this table except CR11 and CR12.** This is the aggregation point. |
|
||||
| CR14 | Coordination-store access (no secret) | Filesystem permissions — `control_plane_db.py:47`, created `0o700` (`control_plane_db.py:380`), opened with a local file lock (`control_plane_db.py:386`) | B6, ADV5 | Full read/write of locks, leases, and decision records (A6). **There is no credential here at all** — anything running as the operator can rewrite ownership. |
|
||||
|
||||
### Findings
|
||||
|
||||
**Finding 1 — Role separation is not credential separation.** Four `prgs` roles resolve to
|
||||
one Gitea account (`sysadmin`): reviewer, merger, reconciler, and controller. A stolen
|
||||
reviewer credential *is* a merger credential. A9 — separation of duty between approving and
|
||||
landing — is therefore enforced entirely by which local process a call reaches (B2), and not
|
||||
at all by the forge. It survives exactly as long as B2 does, and B2 is the boundary the
|
||||
migration dissolves.
|
||||
|
||||
**Finding 2 — The `mdcps` tenant has no role separation at all.** Author, reviewer, and
|
||||
merger all resolve to account `913443`. One credential can open a PR, approve it, and merge
|
||||
it. The in-process self-review check compares the authenticated username against the PR
|
||||
author and would refuse — but that check runs on our side of B4. It is not a property of
|
||||
the credential, and an attacker holding the token does not call our tools.
|
||||
|
||||
**Finding 3 — Any one role process can resolve every other role's credential.** This is not
|
||||
inferred; it is demonstrated by tool output. `gitea_list_profiles`
|
||||
(`gitea_mcp_server.py:19258`) called from the **author** session reports
|
||||
`identity_status: "credentials present"` for `prgs-merger`, `prgs-reviewer`,
|
||||
`prgs-reconciler`, and every `mdcps` profile, because it calls `resolve_token` on each one
|
||||
(`gitea_mcp_server.py:19309`). The author process does not merely *have access to* the
|
||||
merger's credential — it reads it to answer a status query. B2 is not a credential boundary
|
||||
in either direction.
|
||||
|
||||
**Finding 4 — The Gitea server reads CI and observability secrets.** `gitea_audit_config`
|
||||
(`gitea_mcp_server.py:19552`) reports `MDCPS Jenkins: enabled, read-only, authenticated`.
|
||||
That word `authenticated` is produced by `service_summaries` (`gitea_mcp_server.py:19574`,
|
||||
defined at `gitea_config.py:837`), whose default check calls `_keychain_token` on the
|
||||
service's own keychain reference (`gitea_config.py:851`). Producing that one line requires
|
||||
the Gitea MCP server to read the Jenkins secret and the GlitchTip secret out of the
|
||||
keychain. B7 does not exist.
|
||||
|
||||
**Finding 5 — Jenkins and GlitchTip are already decomposed; the reach is residual.** Their
|
||||
tools live in separately registered servers, marked `external-mcp`
|
||||
(`gitea_mcp_server.py:17707`, `gitea_mcp_server.py:17713`, `gitea_mcp_server.py:17734`,
|
||||
`gitea_mcp_server.py:17739`) with their own expected tool sets (`mcp_discoverability.py:9`,
|
||||
`mcp_discoverability.py:17`). The correct decomposition was already chosen. What remains is
|
||||
a leak across it: the credential *references* still live in the Gitea configuration and are
|
||||
still resolved by the Gitea process. #75 bundled these services into one control-plane
|
||||
umbrella; the tools were separated afterwards, the credentials were not.
|
||||
|
||||
**Finding 6 — Sentry is the exception that is not decomposed.** Unlike Jenkins and
|
||||
GlitchTip, the Sentry bridge runs *inside* the Gitea server, resolving its token from the
|
||||
process environment (`sentry_incident_bridge.py:190`) and sending it as a bearer header
|
||||
(`sentry_incident_bridge.py:289`). Being an environment variable rather than a keychain item
|
||||
makes it strictly worse: it needs no keychain prompt and is inherited by every subprocess the
|
||||
server spawns — including the `ps` invocations at `gitea_mcp_server.py:21462` and
|
||||
`gitea_mcp_server.py:21506`, reached from `gitea_mcp_server.py:21442`.
|
||||
|
||||
**Finding 7 — The highest-value coordination asset has the weakest gate.** A6 is protected
|
||||
by filesystem permissions alone (CR14). Corrupting a lease requires no Gitea credential,
|
||||
produces no forge-side audit record, and breaks the mutual exclusion the entire workflow
|
||||
assumes. Every other asset costs an attacker a credential; this one costs nothing beyond
|
||||
local access, which is exactly ADV5's position.
|
||||
|
||||
**Finding 8 — Provenance authenticates the launch, not the caller.** `server_provenance` is
|
||||
reported as exactly `client_managed` or `manual_launch` (`gitea_mcp_server.py:19001`),
|
||||
derived from environment inspection (`gitea_mcp_server.py:15412`) with the recognized-key
|
||||
allowlist at `gitea_config.py:1172` and the generator that emits the marker at
|
||||
`gitea_config.py:1233`. Every one of those facts is fixed at process start. A client that is
|
||||
trustworthy at launch and compromised a minute later remains `client_managed` for the life
|
||||
of the process, and the stdio contract that underwrites it is stated as a property of the
|
||||
server itself (`mcp_server.py:4`).
|
||||
|
||||
## 6. Decomposition ruling
|
||||
|
||||
This section is the ruling #956 requires. It is a decision, not a recommendation.
|
||||
|
||||
**D1 — No unrelated co-residency.** A single integration process **must not** hold, resolve,
|
||||
or be able to resolve credentials for services it does not itself integrate with.
|
||||
Concretely: the Gitea MCP service may hold Gitea credentials and nothing else. Jenkins,
|
||||
GlitchTip, Sentry, and any database credential are **not permitted** to co-reside with Gitea
|
||||
credentials in one process.
|
||||
|
||||
*Rationale.* A process is the smallest unit an attacker takes whole. Once ADV1 or ADV2
|
||||
controls execution in a process, every credential that process can resolve is theirs, and no
|
||||
in-process check helps, because the checks are in the process too. Blast radius is therefore
|
||||
a property of the process boundary and nothing finer. Findings 4 and 6 show that today one
|
||||
compromise of the Gitea server yields CI read access, error-tracking read access, and — via
|
||||
CR13 — every role credential on both tenants. That is the single largest reduction in blast
|
||||
radius available anywhere in epic #929, and it costs no new mechanism: the decomposition
|
||||
already exists (Finding 5) and is merely leaked across.
|
||||
|
||||
**D2 — Separation of duty must be backed by credentials.** Two roles whose separation is a
|
||||
security property must not resolve to the same forge account. Specifically, reviewer and
|
||||
merger must be distinct accounts. Today they are not, on either tenant (Findings 1 and 2).
|
||||
|
||||
*Rationale.* B2 is a process boundary, and the migration's entire purpose is to replace
|
||||
process boundaries with request-level ones. A separation enforced only by which process a
|
||||
call reaches does not survive that replacement — and it is already bypassable by anyone who
|
||||
holds the token and calls the API instead of the tool.
|
||||
|
||||
**D3 — Credential resolution is scoped to the request principal.** A session must resolve its
|
||||
own credential and must have no path to any other principal's. The resolve-every-profile
|
||||
behavior behind `gitea_mcp_server.py:19309` and `gitea_mcp_server.py:19574` must report
|
||||
configured-or-not from configuration alone, without resolving the secret.
|
||||
|
||||
*Rationale.* Finding 3. An audit surface that proves a credential exists by fetching it is a
|
||||
credential-aggregation primitive wearing a diagnostic's clothes.
|
||||
|
||||
**D4 — Coordination state is a protected asset with its own authority.** Access to locks,
|
||||
leases, and decision records must require an authenticated session, not merely local
|
||||
filesystem access.
|
||||
|
||||
*Rationale.* Finding 7. #937 already moves this store for concurrency reasons; the
|
||||
authorization requirement must land with it, or the store becomes remotely reachable while
|
||||
still being authorized by nothing.
|
||||
|
||||
### Exceptions
|
||||
|
||||
**One, time-boxed.** During the dual-run window defined by #939, the **local** stdio fleet
|
||||
may continue to resolve Jenkins and GlitchTip credential *references* from the shared
|
||||
configuration, because removing them from the local configuration is not a prerequisite for
|
||||
standing up the remote endpoint and would strand the operator's existing local workflow.
|
||||
|
||||
This exception is bounded by all of:
|
||||
|
||||
- It applies to the local stdio deployment only. The remote endpoint (#938) must be
|
||||
configured with Gitea credentials and no others from its first day.
|
||||
- It expires when #939 completes. It does not survive cutover.
|
||||
- It does not extend to Sentry: CR11 and CR12 are process-environment credentials in the
|
||||
Gitea server (Finding 6) and must be absent from the remote deployment's environment
|
||||
regardless of dual-run state.
|
||||
|
||||
No exception is granted to D2, D3, or D4.
|
||||
|
||||
### Consequences for the target architecture
|
||||
|
||||
- The remote endpoint serves **Gitea only**. It is not a general control-plane endpoint.
|
||||
- Jenkins and GlitchTip keep their existing separate servers, and their credential
|
||||
references move out of the Gitea configuration.
|
||||
- The Sentry bridge either moves behind its own service boundary or is absent from the
|
||||
remote deployment. It does not travel with the Gitea server.
|
||||
- Reviewer and merger accounts diverge before the endpoint is trusted for merges, or A9 is
|
||||
recorded as unenforced.
|
||||
|
||||
## 7. Child-to-boundary mapping
|
||||
|
||||
Every #929 child from 2 through 10, mapped to the boundary it implements. A child
|
||||
implementing more than one boundary names its primary first.
|
||||
|
||||
| Child | Issue | Boundaries | What it must establish | Rulings it must honor |
|
||||
| ----: | ----- | ---------- | ---------------------- | --------------------- |
|
||||
| 2 | #931 | B1, B9 | The bound transport becomes a validated value that provenance and freshness can both key on. Without it neither B1 nor B9 has an input. | — |
|
||||
| 3 | #932 | B2 | The role becomes a property of the request, not the process — the boundary the migration otherwise deletes. | D2, D3 |
|
||||
| 4 | #933 | B3, B7 | Credentials come from a provider keyed by principal. This is where D1 and D3 are either enforced or permanently lost. | D1, D3 |
|
||||
| 5 | #934 | B1 | Session provenance replaces pipe-and-process-table proof with an authenticated session identity. | — |
|
||||
| 6 | #935 | B9 | Freshness redefined against deployed build identity, with an explicit undeterminable verdict. | — |
|
||||
| 7 | #936 | B5 | Every tool classified and the filesystem boundary enforced at dispatch, so a tool cannot return green about the wrong disk. | — |
|
||||
| 8 | #937 | B6 | One authority per ownership question, with session-identity liveness and atomic transitions. | D4 |
|
||||
| 9 | #938 | B4, B1, B8 | The endpoint: authentication, principal binding, transport security, and — critically — the deployed credential set. | D1, D2, D3 |
|
||||
| 10 | #939 | B6 | Dual-run with exactly one coordination authority at every instant, and the rollback that proves the way back. | D1 exception expiry |
|
||||
|
||||
Boundary coverage: B1 (#931, #934, #938), B2 (#932), B3 (#933), B4 (#938), B5 (#936),
|
||||
B6 (#937, #939), B7 (#933), B8 (#938), B9 (#931, #935).
|
||||
|
||||
B7 has exactly one owner, #933, and that is deliberate. B7 is not created by standing up an
|
||||
endpoint; it is created by deciding which credentials a process may resolve, which is
|
||||
precisely what the credential-provider child does.
|
||||
|
||||
## 8. Adversarial walkthrough: a fully compromised LLM client
|
||||
|
||||
The required negative case. ADV1 holds one MCP client and issues arbitrary tool calls. What
|
||||
does it reach at each boundary?
|
||||
|
||||
### Before the migration
|
||||
|
||||
The client is attached to the local fleet over stdio.
|
||||
|
||||
| Boundary | What ADV1 reaches | Stopped by |
|
||||
| -------- | ----------------- | ---------- |
|
||||
| B1 | Everything the fleet serves. The client *is* the sanctioned launcher: it satisfies the client-managed check (`gitea_mcp_server.py:15412`) by construction, and provenance is never re-verified after launch (Finding 8). | Nothing. The guard authenticates the launch, not the caller. |
|
||||
| B2 | All five roles — it is attached to all five namespaces. It can author a PR, approve it from the reviewer namespace, and merge it from the merger namespace. | Only the in-process self-review check, which compares `jcwalker3` (author) against `sysadmin` (reviewer) and **passes**, because Finding 1 made them different accounts while leaving reviewer and merger identical. A9 falls in one sequence of legitimate calls. |
|
||||
| B3 | Every credential in CR1–CR10 via CR13, with no additional prompt — the daemon is already sanctioned, so `assert_keychain_access_allowed` (`mcp_daemon_guard.py:440`) returns immediately. | Nothing. |
|
||||
| B4 | A1 and A2 in full. | Branch protection at the forge, to the extent configured. |
|
||||
| B5 | The operator's checkout and every worktree, through the author tools (`gitea_mcp_server.py:10894`), plus the shared stderr path at `mcp_server.py:10`. | Nothing; the server's disk is the target disk. |
|
||||
| B6 | All coordination state — no credential required (CR14). It can forge lease ownership and clear decision locks. | Filesystem permissions, which it already satisfies. |
|
||||
| B7 | Jenkins (A4) and GlitchTip (A5) secrets via Finding 4, and CR11/CR12 from its own environment. | Nothing. |
|
||||
| B8 | Both tenants. | Nothing in-process; only the disabled-context check (`gitea_config.py:704`), which does not apply to enabled contexts. |
|
||||
| B9 | Can it run stale code? Only by advancing the checkout, which parity detects (`master_parity_gate.py:255`). | This gate holds — the one boundary that behaves as designed against ADV1. |
|
||||
|
||||
**Result: total.** One compromised client yields A1 through A8 and A10. The only asset with
|
||||
real resistance is A1 via branch protection, and the client holds the merger credential
|
||||
anyway. Nine boundaries, one meaningful stop.
|
||||
|
||||
### After the migration
|
||||
|
||||
The same client authenticates to the remote endpoint with one role's credential, assuming
|
||||
#931–#939 land **and honor D1–D4**.
|
||||
|
||||
| Boundary | What ADV1 reaches | Stopped by |
|
||||
| -------- | ----------------- | ---------- |
|
||||
| B1 | One authenticated session, bound to one principal. | #934: a forged or expired session identity is refused; the client cannot mint one. |
|
||||
| B2 | **One role.** Presenting the author credential yields author permissions only. | #932: the principal comes from the credential, not from which endpoint was reached. |
|
||||
| B3 | **One credential — its own.** | #933 with D3: the provider resolves by principal, and no diagnostic resolves the others. |
|
||||
| B4 | That role's authority on the forge. | Endpoint authentication (#938); plaintext and unauthenticated attempts refused before dispatch. |
|
||||
| B5 | **Nothing.** Filesystem tools are refused over the remote transport with a named blocker. | #936. |
|
||||
| B6 | Its own leases; contention resolves to exactly one winner. | #937 with D4: authenticated session required, not filesystem access. |
|
||||
| B7 | **Nothing.** No CI or observability credential exists in the process. | D1 — the single largest reduction on this table. |
|
||||
| B8 | One tenant. | D1 and #938: the deployment carries one tenant's credentials. |
|
||||
| B9 | Cannot induce stale enforcement. | #935: explicit fail-closed verdict, including undeterminable. |
|
||||
|
||||
**Result: bounded.** The compromise is contained to one role on one tenant, with no
|
||||
filesystem reach and no lateral credential access. A9 survives *only if D2 lands* — if
|
||||
reviewer and merger still share `sysadmin`, a compromised reviewer session still merges, and
|
||||
this row reads the same after the migration as before it.
|
||||
|
||||
### What the migration does not fix
|
||||
|
||||
Against **ADV2**, both tables are identical. Prompt injection does not need to cross a
|
||||
boundary: it arrives inside an authorized session and asks that session to do what it is
|
||||
already permitted to do. Every "stopped by" above authenticates a principal, and the
|
||||
injected instruction has the correct principal. The migration reduces ADV1's blast radius by
|
||||
roughly an order of magnitude and reduces ADV2's by nothing.
|
||||
|
||||
The controls that do constrain ADV2 are per-principal permission scope (#932), repository
|
||||
scope (`gitea_config.py:499`), and credential-backed separation of duty (D2) — each limiting
|
||||
what an authenticated session may do *regardless of what it is asked for*. #955's
|
||||
secure-isolation end state should be read with that distinction in mind: removing credentials
|
||||
from clients defeats ADV1 and ADV5, and does not by itself defeat ADV2.
|
||||
|
||||
Two further items are explicitly out of scope here and unowned by #929:
|
||||
|
||||
- **Session-credential rotation and revocation.** #938 names rotation as documentation, but
|
||||
no child owns proving that a revoked credential stops an in-flight session.
|
||||
- **ADV3** (malicious tool arguments) is diffused across every child rather than owned. The
|
||||
per-request principal work in #932 is the natural place to assert that identifiers taken
|
||||
from the request never authorize anything on their own.
|
||||
|
||||
## 9. How to verify this document
|
||||
|
||||
1. `PYTHONPATH=. pytest tests/test_issue_956_threat_model.py` — resolves every anchor
|
||||
against the working tree and checks the document's structural obligations.
|
||||
2. Pick any five anchors at random and read them; the fixture states what each line must
|
||||
contain.
|
||||
3. Reproduce Findings 3 and 4 live: call `gitea_list_profiles` and `gitea_audit_config`
|
||||
from the **author** namespace. Credential presence reported for roles other than the
|
||||
active one is Finding 3; `MDCPS Jenkins: enabled, read-only, authenticated` is Finding 4.
|
||||
|
||||
If the anchor test fails after an unrelated refactor, the anchors moved and the fixture
|
||||
needs regenerating — the claims are still true, but they are no longer traceable, which
|
||||
#956 treats as the same defect.
|
||||
@@ -98,6 +98,8 @@ already define, and a regression test asserts each mapping matches.
|
||||
| `system.rebind_session_worktree` | operator | gated_write | `gitea.read` | Yes | No | No | 2 |
|
||||
| `system.reconcile_cleanups` | controller | privileged | `gitea.pr.close` | Yes | No | No | 2 |
|
||||
| `initiate_workflow` | operator | gated_write | `gitea.read` | Yes | No | No | 2 |
|
||||
| `observability_reconcile_incident` | operator | gated_write | `gitea.read` | Yes | No | No | 4 |
|
||||
| `observability_link_issue` | operator | gated_write | `gitea.read` | Yes | No | No | 4 |
|
||||
|
||||
**Dual control** means the acting principal may not be the sole authority: a
|
||||
second distinct principal must confirm. **Break-glass** means the action is
|
||||
|
||||
Reference in New Issue
Block a user