fix(webui): authority-aware ownership + redacted contamination text (#641)
Addresses the two blockers from the PR #898 review ata81db754. B1 - degraded ownership inventory was rendered as affirmative absence. _build_session_rows read the sessions/leases/locks sections without consulting their status, so a session row emitted lease_ids=() and worktree_paths=() whether the session genuinely held nothing or the lease store simply could not be read. The renderer printed both as "none" and "unbound", contradicting the ownership_authority_complete invariant documented on InventorySnapshot. SessionRow now carries lease_authority and worktree_authority. A worktree binding is correlated through lease work numbers, so it is unproven when either the leases or the locks section fails to read -- this covers the narrow variant where locks hold real worktree paths but a degraded leases section leaves work_numbers empty. The renderer emits "unknown (inventory <status>)" with an authority-unproven badge instead of none/unbound, the card names the unreadable sections, and an empty session list from an unreadable sessions section no longer reads as "no sessions recorded". snapshot_to_dict exports ownership_authority_complete, ownership_section_status, and per-row lease_authority / worktree_authority so /api/sessions consumers can distinguish the two cases. B2 - contamination payload strings bypassed redaction. _inspect_contamination copied command_summary, session_id, role and reason_class out of the marker payload with only str(), while every inventory-sourced field on the same page arrives through webui.inventory.scrub(). The write-time redactor stable_branch_push_guard.redact_command is a narrow denylist that leaves absolute $HOME paths, -H 'X-Api-Key: <value>', --password <value>, and PRIVATE_KEY=<value> intact, and this is the first web surface to render command_summary at all. Adds webui.inventory.scrub_text(), which collapses $HOME and redacts credential-shaped tokens and URL userinfo anywhere inside a string rather than only at its start, and routes the marker payload through it. scrub() and every existing caller are untouched. The command_summary field is kept: it is the #630 evidence naming which daemon was killed. The module docstring claiming absolute paths were already collapsed is corrected. Also: removes the locks_by_session_hint dead loop and its discard (N1), adds the missing trailing newline to webui/runtime_views.py (N4), drops an unused dataclasses.field import, and documents both honesty rules in docs/webui-local-dev.md. Tests: tests/test_webui_sessions_view.py grows from 12 to 26 cases, covering degraded and unavailable ownership sections in both the HTML and JSON paths, the locks-readable/leases-degraded variant, a guard against over-correcting clean inventory into "unknown", the previously untested expired-lease flag, HTML escaping of hostile values in clean and degraded renders, and each secret class the write-time denylist misses. The STATUS_UNAVAILABLE import that was present but unused is now exercised. Validation, from the issue worktree with venv/bin/python (Python 3.14.5, pytest 9.1.1): pytest tests/test_webui_sessions_view.py tests/test_webui_*.py -q -> 512 passed, 376 subtests (was 498 / 372; +14 new tests) pytest tests/test_issue_854_semantic_container_exclusion.py -q -> 13 passed, 8 subtests 13-file runtime/health/inventory/restart set -> 1 failed, 223 passed; the single failure is test_runtime_clarity.py::TestRuntimeClarity:: test_activate_profile_succeeds_when_enabled, the identical test and assertion the reviewer recorded on master at7af40fb5, so it is baseline-equivalent and not introduced here. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
This commit is contained in:
+105
-16
@@ -12,18 +12,30 @@ Sources:
|
||||
and collision signals from the control-plane DB + filesystem.
|
||||
* :mod:`mcp_session_state` — durable contamination markers (inspect only).
|
||||
|
||||
Secrets are never read. Absolute paths are collapsed by inventory redaction.
|
||||
Secrets are never read. Inventory-sourced values arrive already redacted by
|
||||
:func:`webui.inventory.scrub`; free-form marker text this module loads itself is
|
||||
put through :func:`webui.inventory.scrub_text`, which collapses ``$HOME`` and
|
||||
redacts credential-shaped tokens *inside* a string rather than only at its start.
|
||||
|
||||
Ownership columns are authority-aware. A lease or lock section that could not be
|
||||
read renders as ``unknown``, never as ``none`` or ``unbound``: a lease the reader
|
||||
could not load is not an absent lease (see
|
||||
:attr:`webui.inventory.InventorySnapshot.ownership_authority_complete`).
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from dataclasses import dataclass, field
|
||||
from dataclasses import dataclass
|
||||
from typing import Any, Callable
|
||||
|
||||
import mcp_session_state
|
||||
from webui.inventory import (
|
||||
OWNERSHIP_SECTIONS,
|
||||
STATUS_OK,
|
||||
InventorySection,
|
||||
InventorySnapshot,
|
||||
load_inventory_snapshot,
|
||||
scrub_text,
|
||||
snapshot_to_dict as inventory_snapshot_to_dict,
|
||||
)
|
||||
from webui.runtime_health import (
|
||||
@@ -61,6 +73,29 @@ _CONTAMINATION_KINDS: tuple[str, ...] = (
|
||||
mcp_session_state.KIND_STABLE_BRANCH_CONTAMINATION,
|
||||
)
|
||||
|
||||
#: Status recorded on a row when the backing inventory section is absent
|
||||
#: entirely — distinct from a section that reported itself degraded.
|
||||
AUTHORITY_MISSING = "missing"
|
||||
|
||||
|
||||
def _section_status(section: InventorySection | None) -> str:
|
||||
"""Status of an ownership section, treating an absent section as missing."""
|
||||
if section is None:
|
||||
return AUTHORITY_MISSING
|
||||
return section.status
|
||||
|
||||
|
||||
def _combined_authority(*statuses: str) -> str:
|
||||
"""Worst status of the sections a derived column depends on.
|
||||
|
||||
A column proved from two sections is only trustworthy when *both* read
|
||||
cleanly, so the first non-``ok`` status wins.
|
||||
"""
|
||||
for status in statuses:
|
||||
if status != STATUS_OK:
|
||||
return status
|
||||
return STATUS_OK
|
||||
|
||||
|
||||
@dataclass(frozen=True)
|
||||
class ContaminationMarker:
|
||||
@@ -109,6 +144,17 @@ class SessionRow:
|
||||
worktree_paths: tuple[str, ...] = ()
|
||||
stale_flags: tuple[str, ...] = ()
|
||||
contamination_flags: tuple[str, ...] = ()
|
||||
#: Status of the section backing ``lease_ids``/``work_refs``. While this is
|
||||
#: not ``ok`` those tuples mean "could not be read", never "none held".
|
||||
lease_authority: str = STATUS_OK
|
||||
#: Worst status across the sections backing ``worktree_paths`` (locks are
|
||||
#: correlated through lease work numbers, so both must read cleanly).
|
||||
worktree_authority: str = STATUS_OK
|
||||
|
||||
@property
|
||||
def ownership_authority_complete(self) -> bool:
|
||||
"""True only when this row's ownership columns are provable."""
|
||||
return self.lease_authority == STATUS_OK and self.worktree_authority == STATUS_OK
|
||||
|
||||
def to_dict(self) -> dict[str, Any]:
|
||||
return {
|
||||
@@ -128,6 +174,18 @@ class SessionRow:
|
||||
"contamination_flags": list(self.contamination_flags),
|
||||
"is_stale": bool(self.stale_flags),
|
||||
"is_contaminated": bool(self.contamination_flags),
|
||||
"lease_authority": self.lease_authority,
|
||||
"worktree_authority": self.worktree_authority,
|
||||
"lease_authority_complete": self.lease_authority == STATUS_OK,
|
||||
"worktree_authority_complete": self.worktree_authority == STATUS_OK,
|
||||
"ownership_authority_complete": self.ownership_authority_complete,
|
||||
"ownership_note": (
|
||||
"Lease and worktree columns are proved from sections that read "
|
||||
"cleanly."
|
||||
if self.ownership_authority_complete
|
||||
else "An ownership source could not be read; empty lease_ids or "
|
||||
"worktree_paths on this row mean unknown, not unowned."
|
||||
),
|
||||
}
|
||||
|
||||
|
||||
@@ -154,6 +212,19 @@ class SessionViewSnapshot:
|
||||
def active_contamination(self) -> tuple[ContaminationMarker, ...]:
|
||||
return tuple(m for m in self.contamination_markers if m.to_dict()["active"])
|
||||
|
||||
@property
|
||||
def ownership_authority_complete(self) -> bool:
|
||||
"""False while any ownership section is degraded, absent, or unavailable."""
|
||||
return self.inventory.ownership_authority_complete
|
||||
|
||||
@property
|
||||
def ownership_section_status(self) -> dict[str, str]:
|
||||
"""Per-section status for the three ownership-bearing sections."""
|
||||
return {
|
||||
name: _section_status(self.inventory.section(name))
|
||||
for name in OWNERSHIP_SECTIONS
|
||||
}
|
||||
|
||||
|
||||
def _inspect_contamination(
|
||||
kind: str,
|
||||
@@ -180,7 +251,7 @@ def _inspect_contamination(
|
||||
role = None
|
||||
command_summary = None
|
||||
cleared = False
|
||||
summary = str(envelope.get("summary") or "")
|
||||
summary = scrub_text(str(envelope.get("summary") or ""))
|
||||
|
||||
if envelope.get("on_disk") and envelope.get("has_payload"):
|
||||
try:
|
||||
@@ -194,20 +265,25 @@ def _inspect_contamination(
|
||||
command_summary = payload.get("command_summary") or payload.get("detail")
|
||||
cleared = bool(payload.get("cleared_by_reconciler"))
|
||||
if not summary:
|
||||
summary = (
|
||||
summary = scrub_text(
|
||||
f"{kind}: {reason_class or 'present'}"
|
||||
+ (" (cleared)" if cleared else "")
|
||||
)
|
||||
|
||||
# Marker payloads are operator-supplied free text and are the one thing on
|
||||
# this page that does not arrive through inventory scrubbing. The write-time
|
||||
# redactor is a narrow denylist, so redact again at the display boundary:
|
||||
# it leaves $HOME paths, `-H 'X-Api-Key: …'`, `--password …`, and
|
||||
# `PRIVATE_KEY=…` intact. The field itself stays — it is #630 evidence.
|
||||
return ContaminationMarker(
|
||||
kind=kind,
|
||||
on_disk=bool(envelope.get("on_disk")),
|
||||
has_payload=bool(envelope.get("has_payload")),
|
||||
summary=summary or f"{kind}: not present",
|
||||
reason_class=str(reason_class) if reason_class else None,
|
||||
session_id=str(session_id) if session_id else None,
|
||||
role=str(role) if role else None,
|
||||
command_summary=str(command_summary) if command_summary else None,
|
||||
reason_class=scrub_text(str(reason_class)) if reason_class else None,
|
||||
session_id=scrub_text(str(session_id)) if session_id else None,
|
||||
role=scrub_text(str(role)) if role else None,
|
||||
command_summary=scrub_text(str(command_summary)) if command_summary else None,
|
||||
cleared=cleared,
|
||||
)
|
||||
|
||||
@@ -232,18 +308,21 @@ def _build_session_rows(
|
||||
leases_section = inventory.section("leases")
|
||||
locks_section = inventory.section("locks")
|
||||
|
||||
# Ownership columns may only assert absence when their source read cleanly.
|
||||
lease_authority = _section_status(leases_section)
|
||||
# Locks carry claimant profile/username, not control-plane session ids, so a
|
||||
# worktree binding is correlated through lease work numbers: it depends on
|
||||
# the locks *and* the leases section.
|
||||
worktree_authority = _combined_authority(
|
||||
lease_authority, _section_status(locks_section)
|
||||
)
|
||||
|
||||
leases_by_session: dict[str, list[dict[str, Any]]] = {}
|
||||
for lease in (leases_section.items if leases_section else ()):
|
||||
sid = str(lease.get("session_id") or "")
|
||||
if sid:
|
||||
leases_by_session.setdefault(sid, []).append(lease)
|
||||
|
||||
locks_by_session_hint: dict[str, list[dict[str, Any]]] = {}
|
||||
for lock in (locks_section.items if locks_section else ()):
|
||||
# Locks carry claimant profile/username, not control-plane session ids.
|
||||
# Correlate later by matching lease work_number ↔ lock issue_number.
|
||||
pass
|
||||
|
||||
active_markers = [m for m in contamination if m.to_dict()["active"]]
|
||||
marker_session_ids = {
|
||||
m.session_id for m in active_markers if m.session_id
|
||||
@@ -324,11 +403,11 @@ def _build_session_rows(
|
||||
worktree_paths=tuple(worktree_paths),
|
||||
stale_flags=tuple(dict.fromkeys(stale_flags)),
|
||||
contamination_flags=tuple(dict.fromkeys(contamination_flags)),
|
||||
lease_authority=lease_authority,
|
||||
worktree_authority=worktree_authority,
|
||||
)
|
||||
)
|
||||
|
||||
# Silence unused variable for the locks_by_session_hint placeholder path.
|
||||
_ = locks_by_session_hint
|
||||
return tuple(rows)
|
||||
|
||||
|
||||
@@ -415,6 +494,16 @@ def snapshot_to_dict(snapshot: SessionViewSnapshot) -> dict[str, Any]:
|
||||
"stale": snapshot.stale_session_count,
|
||||
"contaminated": snapshot.contaminated_session_count,
|
||||
},
|
||||
"ownership_authority_complete": snapshot.ownership_authority_complete,
|
||||
"ownership_section_status": snapshot.ownership_section_status,
|
||||
"ownership_note": (
|
||||
"Every ownership source read cleanly; a session with no lease and no "
|
||||
"worktree path genuinely holds neither."
|
||||
if snapshot.ownership_authority_complete
|
||||
else "An ownership source is degraded or unavailable. Empty lease_ids "
|
||||
"and worktree_paths mean unknown, not unowned; consult each row's "
|
||||
"lease_authority and worktree_authority."
|
||||
),
|
||||
"contamination_markers": [
|
||||
marker.to_dict() for marker in snapshot.contamination_markers
|
||||
],
|
||||
|
||||
Reference in New Issue
Block a user