diff --git a/cli/hub_server.py b/cli/hub_server.py index b894a0f..cacca1c 100644 --- a/cli/hub_server.py +++ b/cli/hub_server.py @@ -3259,6 +3259,85 @@ def api_hub_override_costs(): "project": project_path}) +@app.route("/api/hub/overrides/policy", methods=["GET"]) +def api_hub_override_policy_get(): + """The effective `override` section for one project. + + WHY THE HUB NEEDS THIS. The hub could already LIST override requests and + DECIDE them, but not say whether requests may exist at all — that lives in + `override.enabled`, which was reachable only from the phone API or by hand- + editing `.c3/config.json`. So the hub could answer a question the agent was + never allowed to ask, and the only way to change that was a text editor. + + Registered BEFORE the POST-only ```` route for the same reason + ``costs`` is: a GET on ``policy`` must never fall through to a 405 there. + """ + from services import override_policy as opol + raw = (request.args.get("path") or "").strip() + if not raw: + return jsonify({"error": "path is required"}), 400 + try: + project_path = str(_resolve_project_path(raw)) + except ValueError as e: + return jsonify({"error": str(e)}), 404 + policy = opol.resolve(project_path) + return jsonify({ + "path": project_path, + "project_name": Path(project_path).name, + "policy": policy.as_dict(), + "layers": list(opol.LAYER_KEYS), + "typed_confirm_layers": sorted(opol.TYPED_CONFIRM_LAYERS), + "hard_max_ttl_s": opol.HARD_MAX_TTL_S, + # Verbatim from the spec, and the one thing a toggle screen must not + # let the reader forget: policy is a floor, not a promise. + "coverage_note": ( + "Project and global policy merge by tightening only: a layer is " + "escalatable iff both scopes allow it, and the numeric limits take " + "the smaller value." + ), + }) + + +@app.route("/api/hub/overrides/policy", methods=["POST"]) +def api_hub_override_policy_set(): + """Edit one project's `override` section from the hub. + + Widening — enabling the feature or turning a layer on — needs + ``confirm: "widen"``. Tightening never does: making the guard stricter is + always allowed to be one click. + + Validation, widening detection and the write are + ``services.override_policy``'s, shared verbatim with the phone route. Two + implementations of "what counts as widening" would be two chances for one + surface to allow what the other refuses, and this is the check that decides + whether an action needs a typed confirmation. + + ``wake`` is accepted here and refused on the phone: it names a command this + machine runs, and the hub is a desktop surface on this box. + """ + from services import override_policy as opol + data = request.get_json(silent=True) or {} + raw = (data.get("path") or "").strip() + if not raw: + return jsonify({"error": "path is required"}), 400 + try: + project_path = str(_resolve_project_path(raw)) + except ValueError as e: + return jsonify({"error": str(e)}), 404 + try: + written, widens = opol.apply_section( + project_path, data.get("override"), + confirmed=(data.get("confirm") == "widen"), + allow_wake=True) + except opol.PolicyEditError as exc: + return jsonify(exc.payload), exc.status + except (OSError, ValueError) as exc: + return jsonify({"error": f"{type(exc).__name__}: {exc}"}), 500 + return jsonify({"path": project_path, "written": written, + "policy": opol.resolve(project_path).as_dict(), + "widened": widens}) + + @app.route("/api/hub/overrides/", methods=["POST"]) def api_hub_override_decide(request_id): """Approve (minting a grant) or deny one request. decided_by='desktop'. diff --git a/cli/hub_ui/components/hub_access.js b/cli/hub_ui/components/hub_access.js index ee023c4..25d5631 100644 --- a/cli/hub_ui/components/hub_access.js +++ b/cli/hub_ui/components/hub_access.js @@ -482,6 +482,187 @@ function AccGrantsPanel({ projects }) { } // ── The tab ──────────────────────────────────────────────────────────────── +// ─── Override policy (the switch, not the requests) ─────────────────────── +// +// WHY THIS PANEL EXISTS. This screen could already LIST override requests and +// DECIDE them — but not say whether requests may exist at all. That lives in +// `override.enabled`, and it was reachable only from the phone API or by hand- +// editing .c3/config.json. So the hub answered a question the agent was never +// allowed to ask, and the way to change that was a text editor. +// +// THE ONE RULE HERE: widening is never silent. Enabling the feature, or +// turning a layer on, loosens what a single approval can allow. The server +// refuses those without confirm:"widen"; this panel asks in plain words BEFORE +// sending, rather than surfacing a 400 afterwards. Tightening goes straight +// through — making the guard stricter should always be one click. + +// Plain-language layer names. The raw keys are accurate and mean nothing to +// someone deciding whether to tick a box. +const ACC_LAYER_NAMES = { + discipline: 'Discipline holds (native-tool nudges)', + access_readonly: 'Read-only paths — e.g. .git/**, .c3/**', + access_deny: 'Denied paths', + access_builtin: 'Built-in guards', + access_confirm: 'Confirm holds — agent-config files', + mask: 'Masked paths', + shell_warn: 'Shell warnings', +}; + +function accLayerName(key) { return ACC_LAYER_NAMES[key] || key; } + +function AccPolicyPanel({ projects }) { + const [path, setPath] = useState(''); + const [data, setData] = useState(null); + const [draft, setDraft] = useState(null); // {enabled, layers} + const [err, setErr] = useState(''); + const [msg, setMsg] = useState(''); + const [busy, setBusy] = useState(false); + + const load = useCallback(async (p) => { + if (!p) { setData(null); setDraft(null); setErr(''); setMsg(''); return; } + try { + const d = await api.get(`/api/hub/overrides/policy?path=${encodeURIComponent(p)}`); + setData(d); + setDraft({ + enabled: !!(d.policy || {}).enabled, + layers: { ...((d.policy || {}).layers || {}) }, + }); + setErr(''); setMsg(''); + } catch (e) { setData(null); setDraft(null); setErr(apiErr(e)); } + }, []); + + // Mirrors the server's own rule so the question is asked before the request, + // not after the refusal. + const widenings = () => { + if (!data || !draft) return []; + const cur = data.policy || {}; + const out = []; + if (draft.enabled && !cur.enabled) out.push('enable override requests'); + Object.entries(draft.layers || {}).forEach(([k, want]) => { + if (want && !(cur.layers || {})[k]) out.push(`allow asking about: ${accLayerName(k)}`); + }); + return out; + }; + + const save = async () => { + if (!draft || !path) return; + const widens = widenings(); + if (widens.length) { + const ok = window.confirm( + `This LOOSENS the guard:\n\n • ${widens.join('\n • ')}\n\n` + + 'A yes still only lets the agent ASK — every request needs your ' + + 'approval, and a grant is single-use and path-exact.\n\nApply?'); + if (!ok) { setMsg('Unchanged.'); return; } + } + setBusy(true); + try { + const d = await api.post('/api/hub/overrides/policy', { + path, + override: { enabled: draft.enabled, layers: draft.layers }, + confirm: widens.length ? 'widen' : undefined, + }); + setData(d); + setDraft({ + enabled: !!(d.policy || {}).enabled, + layers: { ...((d.policy || {}).layers || {}) }, + }); + setErr(''); + setMsg((d.widened && d.widened.length) + ? `Saved — widened: ${d.widened.join(', ')}` + : 'Saved.'); + } catch (e) { setErr(apiErr(e)); } + setBusy(false); + }; + + const typed = new Set((data && data.typed_confirm_layers) || []); + const dirty = !!(data && draft) && ( + draft.enabled !== !!(data.policy || {}).enabled || + Object.entries(draft.layers || {}).some( + ([k, v]) => !!v !== !!((data.policy || {}).layers || {})[k])); + + return ( +
+
+
May agents ask?
+ + + off by default, per project + +
+ + {err &&
{err}
} + + {data && draft && ( +
+ + +
+
Escalatable layers
+ {(data.layers || []).map(k => ( + + ))} +
+ +
+ {data.coverage_note} +
+ Never escalatable at any setting: the credential vault, + {' '}.c3/secrets.enc, + {' '}.c3/cred_state.json, the dispatcher + fail-closed deny, and the catastrophic shell blocks. +
+ +
+ + {msg && {msg}} +
+
+ )} +
+ ); +} + function HubAccess({ projects }) { const [rows, setRows] = useState([]); const [loaded, setLoaded] = useState(false); @@ -559,6 +740,8 @@ function HubAccess({ projects }) { + + {!loaded ? (
Loading…
) : rows.length === 0 ? ( diff --git a/oracle/services/mobile_api.py b/oracle/services/mobile_api.py index 2e2e752..19081a6 100644 --- a/oracle/services/mobile_api.py +++ b/oracle/services/mobile_api.py @@ -2332,7 +2332,7 @@ def mobile_overrides_policy_set(): section = data.get("override") if not isinstance(section, dict): return jsonify({"error": "body needs an 'override' object"}), 400 - if _WAKE_KEY in section: + if _WAKE_KEY in section: # noqa: SIM102 — kept for the phone-specific copy # `wake` is an argv this machine will execute. Everything else on this # route widens what a tap can approve; this one would decide what runs # when it does. A bearer token from a phone is authentication, not @@ -2387,7 +2387,23 @@ def mobile_overrides_policy_set(): def _override_widenings(current, section: dict) -> list: - """Which requested changes LOOSEN the policy. Names, for the challenge.""" + """Which requested changes LOOSEN the policy. Names, for the challenge. + + DELEGATES to ``services.override_policy.widenings`` since 2.129.2, when the + hub gained its own Security screen. Two copies of "what counts as widening" + is two chances for one surface to quietly allow something the other + refuses — and this is the check that decides whether a tap needs a typed + confirmation. The name is kept because the mobile route tests call it. + """ + from services import override_policy as _opol # noqa: PLC0415 + + return _opol.widenings(current, section) + + +def _override_widenings_legacy(current, section: dict) -> list: + """Retired inline copy, kept only as the reference the extraction came + from; ``tests/test_override_policy_shared.py`` asserts it agrees with the + service so the move can be shown to have preserved behaviour.""" out = [] if section.get("enabled") and not current.enabled: out.append("enabled") @@ -2416,10 +2432,18 @@ def _override_widenings(current, section: dict) -> list: def _write_override_section(project: Path, section: dict) -> dict: """Merge *section* into the project's `.c3/config.json` `override` block. - Project scope only. A global override policy governs every project on the - machine, and the phone has no affordance for reviewing that blast radius — - same call `mobile_enforcement_set` makes about machine-wide discipline. + DELEGATES to ``services.override_policy.write_section`` since 2.129.2 so + the hub's Security screen and the phone write the file the same way — same + merge semantics for `layers`, same atomic publish. The name is kept because + the mobile route tests patch it. """ + from services import override_policy as _opol # noqa: PLC0415 + + return _opol.write_section(project, section) + + +def _write_override_section_legacy(project: Path, section: dict) -> dict: + """Retired inline copy; see ``_override_widenings_legacy``.""" cfg_file = Path(project) / ".c3" / "config.json" cfg_file.parent.mkdir(parents=True, exist_ok=True) cfg = {} diff --git a/services/override_policy.py b/services/override_policy.py index e269023..3ce5c3b 100644 --- a/services/override_policy.py +++ b/services/override_policy.py @@ -530,6 +530,153 @@ def resolve_for_path(path) -> OverridePolicy: return resolve(str(root)) if root else DISABLED +class PolicyEditError(ValueError): + """A rejected policy edit. `widens` is set when the reason is a widening. + + Carries the same fields the mobile route already returned so a caller can + render them without re-deriving anything: `payload` is the JSON body, and + `status` the HTTP code the route should use. + """ + + def __init__(self, message: str, *, status: int = 400, + payload: dict | None = None, widens: list | None = None): + super().__init__(message) + self.status = status + self.payload = dict(payload or {"error": message}) + self.widens = list(widens or []) + + +def widenings(current: "OverridePolicy", section: dict) -> list: + """Which requested changes LOOSEN the policy. Names, for the challenge. + + Extracted from ``oracle.services.mobile_api`` in 2.129.2 so the hub's own + Security screen and the phone answer to ONE implementation. Two copies of + "what counts as widening" is two chances for a surface to quietly allow + something the other refuses, and this is the check that decides whether a + tap needs a typed confirmation. + """ + out: list = [] + if section.get("enabled") and not current.enabled: + out.append("enabled") + for key, want in (section.get("layers") or {}).items(): + if want and not current.layers.get(key, False): + out.append(f"layers.{key}") + for key in ("max_ttl_s", "default_uses", "request_ttl_s", + "max_pending_per_session", "max_requests_per_hour", + # A longer TTL or a longer idle window both mean a rule grant + # outlives more of the conversation, which is a loosening even + # though nothing about it looks like a permission. + "rule_grant_ttl_s", "rule_grant_idle_s"): + if key in section: + try: + if int(section[key]) > int(getattr(current, key)): + out.append(key) + except (TypeError, ValueError): + pass + if section.get("allow_session_grants") and not current.allow_session_grants: + out.append("allow_session_grants") + if section.get("allow_rule_grants") and not current.allow_rule_grants: + out.append("allow_rule_grants") + return out + + +def write_section(project, section: dict) -> dict: + """Merge *section* into the project's `.c3/config.json` `override` block. + + Project scope only. A global override policy governs every project on the + machine, and no remote surface has an affordance for reviewing that blast + radius. + """ + from services.atomic_json import write_json_atomic # noqa: PLC0415 + + cfg_file = Path(project) / ".c3" / "config.json" + cfg_file.parent.mkdir(parents=True, exist_ok=True) + cfg = {} + if cfg_file.is_file(): + try: + cfg = json.loads(cfg_file.read_text(encoding="utf-8")) or {} + except ValueError as exc: + raise ValueError(f"project config is not valid JSON: {exc}") from exc + if not isinstance(cfg, dict): + raise ValueError("project config is not a JSON object") + block = dict(cfg.get("override") or {}) + for key, value in section.items(): + if key == "layers": + merged = dict(block.get("layers") or {}) + merged.update({k: bool(v) for k, v in (value or {}).items()}) + block["layers"] = merged + else: + block[key] = value + cfg["override"] = block + # Shared with the CLI and the hub, and served from a threaded API, so the + # publish goes through the one race-safe writer rather than a temp name + # every writer of this file would pick. + write_json_atomic(cfg_file, cfg, ensure_ascii=False, trailing_newline=False) + return block + + +#: The one `override` key no REMOTE surface may write: it is an argv this +#: machine will execute. The hub is a desktop surface on localhost, so it is +#: allowed to; the phone is not. Callers pass ``allow_wake``. +WAKE_KEY = "wake" + + +def apply_section(project_path, section: dict, *, confirmed: bool = False, + allow_wake: bool = False) -> tuple[dict, list]: + """Validate, widening-check and write one `override` edit. + + Returns ``(written_block, widens)``. Raises :class:`PolicyEditError` with + the status and body the caller should surface. + + Widening — enabling the feature, turning a layer on, raising a ceiling, or + allowing session grants — needs ``confirmed=True``. Tightening never does: + making the guard stricter is always allowed to be one tap. + """ + if not isinstance(section, dict): + raise PolicyEditError("body needs an 'override' object") + + if WAKE_KEY in section and not allow_wake: + raise PolicyEditError( + "'wake' cannot be set from this surface — it names a command this " + "machine runs. Edit .c3/config.json on the desktop.", + status=403, payload={"error": ( + "'wake' cannot be set from this surface — it names a command " + "this machine runs. Edit .c3/config.json on the desktop."), + "key": WAKE_KEY}) + + unknown = sorted(set(section) - set(DEFAULTS)) + if unknown: + # Unknown keys are a hard error, never a silent no-op. + raise PolicyEditError( + f"unknown override key(s): {', '.join(unknown)}", + payload={"error": f"unknown override key(s): {', '.join(unknown)}", + "valid": sorted(DEFAULTS)}) + + layers = section.get("layers") + if layers is not None: + if not isinstance(layers, dict): + raise PolicyEditError("'layers' must be an object") + bad = sorted(set(layers) - set(LAYER_KEYS)) + if bad: + raise PolicyEditError( + f"unknown layer(s): {', '.join(bad)}", + payload={"error": f"unknown layer(s): {', '.join(bad)}", + "valid": list(LAYER_KEYS)}) + + current = resolve(str(project_path)) + widens = widenings(current, section) + if widens and not confirmed: + raise PolicyEditError( + "this widens what an approval can allow: " + ", ".join(widens), + payload={"error": ("this widens what an approval can allow: " + + ", ".join(widens)), + "widens": widens, + "needs_confirmation": True, "confirm_with": "widen"}, + widens=widens) + + return write_section(project_path, section), widens + + def offer_line(layer_key: str, path, tool: str, op: str) -> str: """The single line appended to a refusal when a layer IS escalatable. diff --git a/tests/test_hub_override_policy_routes.py b/tests/test_hub_override_policy_routes.py new file mode 100644 index 0000000..677e713 --- /dev/null +++ b/tests/test_hub_override_policy_routes.py @@ -0,0 +1,154 @@ +"""The hub can now turn override requests on — and cannot do it silently. + +WHAT WAS MISSING. The hub could already LIST override requests +(`GET /api/hub/overrides`) and DECIDE them +(`POST /api/hub/overrides/`) — but nothing exposed `override.enabled`, +which decides whether a request may exist at all. So the hub answered a +question the agent was never allowed to ask, and the only way to change that +was the phone API or hand-editing `.c3/config.json`. + +WHAT THESE PIN. +1. Widening is refused without a typed confirmation, and tightening is not. + That asymmetry is the security property: making the guard stricter should + always be one click; loosening what a single approval can allow must not be. +2. The hub and the phone share ONE implementation of "what counts as widening" + (`services.override_policy`). Two copies would be two chances for one + surface to allow what the other refuses. +3. `GET .../policy` does not fall through to the POST-only `` + route — the same ordering trap `costs` already carries a comment about. +""" +from __future__ import annotations + +import json +import sys +from pathlib import Path + +import pytest + +ROOT = Path(__file__).resolve().parent.parent +for p in (str(ROOT), str(ROOT / "cli")): + if p not in sys.path: + sys.path.insert(0, p) + +from services import override_policy as opol # noqa: E402 + + +@pytest.fixture +def project(tmp_path): + (tmp_path / ".c3").mkdir() + (tmp_path / ".c3" / "config.json").write_text( + json.dumps({"project_path": str(tmp_path)}), encoding="utf-8") + return tmp_path + + +def _read_override(project) -> dict: + cfg = json.loads((project / ".c3" / "config.json").read_text(encoding="utf-8")) + return cfg.get("override") or {} + + +# ── the shared service: the security rule itself ────────────────────── + + +def test_enabling_is_a_widening_and_needs_confirmation(project): + with pytest.raises(opol.PolicyEditError) as ei: + opol.apply_section(project, {"enabled": True}, confirmed=False) + assert "enabled" in ei.value.widens + assert ei.value.payload.get("needs_confirmation") is True + assert _read_override(project) == {}, "it wrote despite refusing" + + +def test_confirmed_widening_is_written(project): + written, widens = opol.apply_section( + project, {"enabled": True}, confirmed=True) + assert widens == ["enabled"] + assert written["enabled"] is True + assert _read_override(project)["enabled"] is True + + +def test_turning_a_layer_on_is_a_widening(project): + opol.apply_section(project, {"enabled": True}, confirmed=True) + with pytest.raises(opol.PolicyEditError) as ei: + opol.apply_section(project, {"layers": {"access_readonly": True}}, + confirmed=False) + assert "layers.access_readonly" in ei.value.widens + + +def test_tightening_never_needs_confirmation(project): + """The asymmetry that matters: stricter is always one click.""" + opol.apply_section(project, {"enabled": True}, confirmed=True) + written, widens = opol.apply_section( + project, {"enabled": False}, confirmed=False) + assert widens == [] + assert written["enabled"] is False + + +def test_unknown_keys_are_a_hard_error_not_a_silent_noop(project): + with pytest.raises(opol.PolicyEditError) as ei: + opol.apply_section(project, {"nonsense": 1}, confirmed=True) + assert "unknown override key" in str(ei.value) + assert _read_override(project) == {} + + +def test_unknown_layers_are_rejected(project): + with pytest.raises(opol.PolicyEditError) as ei: + opol.apply_section(project, {"layers": {"not_a_layer": True}}, + confirmed=True) + assert "unknown layer" in str(ei.value) + + +def test_wake_is_refused_on_remote_surfaces_and_allowed_on_the_desktop(project): + """`wake` names an argv this machine runs. A bearer token from a phone is + authentication, not physical presence.""" + with pytest.raises(opol.PolicyEditError) as ei: + opol.apply_section(project, {"wake": ["notify-send", "hi"]}, + confirmed=True, allow_wake=False) + assert ei.value.status == 403 + written, _ = opol.apply_section(project, {"wake": ["notify-send", "hi"]}, + confirmed=True, allow_wake=True) + assert written["wake"] == ["notify-send", "hi"] + + +def test_layers_merge_rather_than_replace(project): + opol.apply_section(project, {"enabled": True, + "layers": {"access_confirm": True}}, + confirmed=True) + opol.apply_section(project, {"layers": {"mask": False}}, confirmed=True) + block = _read_override(project) + assert block["layers"]["access_confirm"] is True, "an unrelated layer was dropped" + assert block["layers"]["mask"] is False + + +# ── the extraction preserved the phone's behaviour ──────────────────── + + +@pytest.mark.parametrize("section", [ + {"enabled": True}, + {"layers": {"access_readonly": True, "mask": False}}, + {"max_ttl_s": 899}, + {"allow_session_grants": True}, + {"enabled": False}, +]) +def test_shared_widenings_agree_with_the_retired_inline_copy(project, section): + """The mobile route's inline implementation is kept as + ``_override_widenings_legacy`` precisely so the move can be shown to have + preserved behaviour rather than asserted to have.""" + from oracle.services import mobile_api as mapi + + current = opol.resolve(str(project)) + assert opol.widenings(current, section) == \ + mapi._override_widenings_legacy(current, section) + + +# ── the routes ──────────────────────────────────────────────────────── + + +def test_policy_route_is_registered_before_the_request_id_catchall(): + import cli.hub_server as hub + + rules = [str(r) for r in hub.app.url_map.iter_rules() + if "/api/hub/overrides" in str(r)] + assert "/api/hub/overrides/policy" in rules + # A GET on `policy` must not fall through to the POST-only converter. + match = hub.app.url_map.bind("localhost").match( + "/api/hub/overrides/policy", method="GET") + assert match[0] == "api_hub_override_policy_get", match