fix(editor): resolve preview authorization and CI regressions

This commit is contained in:
Alexandre Teixeira
2026-10-28 00:31:42 +00:00
parent 091364f7e6
commit 4079f12ae6
6 changed files with 166 additions and 9 deletions
+10
View File
@@ -569,6 +569,16 @@ async def auto_name_session(session_manager, sess):
if not first_msg:
return
from src.owner_identity import is_request_sentinel_owner
owner = getattr(sess, "owner", None)
invalid_owner = (
not isinstance(owner, str) or not owner.strip()
or owner != owner.strip() or is_request_sentinel_owner(owner)
)
if invalid_owner:
session_manager.update_session_name(sess.id, fallback_session_title(first_msg))
return
endpoint_url = str(getattr(sess, "endpoint_url", "") or "")
model_name = str(getattr(sess, "model", "") or "")
+3
View File
@@ -3881,6 +3881,9 @@ def evaluate_preview_call(name, args, user_text='', *, allow_execute_code=False,
or bare in BROKERED_JOB_TOOLS
or (contract_required and bare in {'edit_image', 'generate_image'})
or (contract_required and bare == 'create_document' and standalone_code_request(user_text))
# The editor command is conservatively classified as a write;
# document.info is a read even when the user requested no edit.
or (bare == 'run_editor_command' and args.get('op') == 'document.info')
)
)
if family not in authorized and not contextual_revision and not contract_scoped_mutation and not fixture_delete and not offered_private_action:
+12 -1
View File
@@ -77,8 +77,19 @@ def test_dav_client_does_not_follow_redirect_to_internal_host():
pass
# The request must actually have left the building — otherwise an early
# error would make "sink not hit" pass vacuously.
assert public_methods == ["PROPFIND"], "the PROPFIND must reach the public server first"
# Some caldav releases retry the original URL with GET to probe auth
# after the redirect exception. Neither request may reach the sink.
assert public_methods in (["PROPFIND"], ["PROPFIND", "GET"]), public_methods
assert sink_hits == [], "redirect toward an internal host must not be followed"
# Positive control: demonstrate that the sink would see the redirect
# if the session's redirect protection were removed.
client.session.max_redirects = 30
try:
client.request(public_url, "PROPFIND", "")
except Exception:
pass # The sink's response is intentionally not a valid DAV body.
assert sink_hits == ["/leak"], "the test must detect a followed redirect"
finally:
internal.shutdown()
public.shutdown()
+50
View File
@@ -3551,6 +3551,56 @@ def test_every_compactly_offered_preview_tool_has_valid_policy_permitted_call():
assert decision.allowed, f'{name}: {decision.reason}'
def test_photo_editor_tools_require_explicit_turn_family_authorization():
# Positive: permitted photo editor workflow with turn authorization
for tool, args, prompt in (
('run_editor_command', {'op': 'document.info'}, 'inspect the document'),
('run_editor_command', {'op': 'adjustments.apply', 'params': {'type': 'brightness-contrast'}}, 'make it brighter'),
('ai_edit_image', {'op': 'remove-bg'}, 'remove background'),
):
allowed = evaluate_preview_call(
tool, args, prompt,
turn_authorized_families={'photo_editor'},
)
assert allowed.allowed, f"Expected {tool} to be allowed with photo_editor turn authority: {allowed.reason}"
# Negative: unauthorized preview (unauthorized family, empty families, or cross-family) fails closed
for unauth_families in (frozenset(), {'notes'}, {'email'}, {'calendar'}, {'search_browser'}):
denied_info = evaluate_preview_call(
'run_editor_command', {'op': 'document.info'}, 'inspect the document',
turn_authorized_families=unauth_families,
)
assert not denied_info.allowed
assert denied_info.reason == 'write_family_not_authorized'
denied_edit = evaluate_preview_call(
'run_editor_command', {'op': 'adjustments.apply', 'params': {'type': 'brightness-contrast'}},
'make it brighter',
turn_authorized_families=unauth_families,
)
assert not denied_edit.allowed
assert denied_edit.reason == 'write_family_not_authorized'
denied_ai = evaluate_preview_call(
'ai_edit_image', {'op': 'remove-bg'}, 'remove background',
turn_authorized_families=unauth_families,
)
assert not denied_ai.allowed
assert denied_ai.reason == 'write_family_not_authorized'
for prompt in ('inspect the document', "don't edit the image"):
denied = evaluate_preview_call(
'run_editor_command', {'op': 'adjustments.apply', 'params': {'type': 'brightness-contrast'}},
prompt, turn_authorized_families={'photo_editor'},
)
assert not denied.allowed
assert denied.reason == 'write_family_not_authorized'
denied = evaluate_preview_call('manage_notes', {'action': 'add', 'content': 'x'},
'make it brighter', turn_authorized_families={'photo_editor'})
assert not denied.allowed
def test_full_compact_inventory_contains_pipeline_for_exact_session_contract():
from src.turn_contract import resolve_full_inventory_contract
+33 -3
View File
@@ -207,12 +207,42 @@ async def test_default_namespace_preserves_loopback_sidecars(namespaces):
assert "network" not in result["containment"]["enforced"]
def _assert_sandboxed_shell_output(result, expected):
# Some distributions' login profiles invoke systemd-cat via im-config.
# The sandbox intentionally has no host journal socket; retain its exact
# diagnostic in the result and reject every other stderr message.
assert result["exit_code"] == 0, result
assert result["containment"]["mechanism"] == "bubblewrap", result
assert result["containment"]["contained"] is True, result
assert result["containment"]["executed"] is True, result
assert result["teardown"]["dead"] is True, result
stdout, separator, stderr = result["output"].partition("\nSTDERR: ")
assert stdout == expected, result
if separator:
assert stderr and all(
line == "Failed to create stream fd: No such file or directory"
for line in stderr.splitlines()
), result
def test_shell_output_check_rejects_unexpected_diagnostics_and_failed_launch():
result = {
"exit_code": 0, "containment": {"mechanism": "bubblewrap", "contained": True, "executed": True},
"teardown": {"dead": True}, "output": "closed\nSTDERR: unexpected error",
}
with pytest.raises(AssertionError):
_assert_sandboxed_shell_output(result, "closed")
result["output"] = "closed"
result["containment"]["executed"] = False
with pytest.raises(AssertionError):
_assert_sandboxed_shell_output(result, "closed")
async def test_namespace_handshake_closes_model_stdin(namespaces):
result = await subprocess_tools.BashTool().execute(
"if read value; then echo unexpected; else echo closed; fi", {},
)
assert result["output"] == "closed", result
assert result["teardown"]["dead"] is True
_assert_sandboxed_shell_output(result, "closed")
@pytest.mark.parametrize("legacy_name", [True, False])
@@ -240,7 +270,7 @@ async def test_execution_environment_cannot_replace_probed_bwrap(namespaces, mon
assert "containment unavailable" in result["error"]
assert outside.read_text() == "escaped"
else:
assert result["output"] == "contained", result
_assert_sandboxed_shell_output(result, "contained")
assert not outside.exists()
+58 -5
View File
@@ -311,14 +311,67 @@ class _Manager:
pass
def test_auto_title_inherits_chat_effort_when_utility_is_blank(util, monkeypatch):
@pytest.fixture
def title_registered_endpoint(tmp_path, monkeypatch):
import core.database as database
import src.database as compat_database
import src.endpoint_resolver as resolver
from tests.helpers.database import disposable_database
monkeypatch.setenv("AUTH_ENABLED", "true")
monkeypatch.setattr("src.settings.load_settings", lambda: {})
monkeypatch.setattr(resolver, "resolve_url", lambda url: url)
with disposable_database(tmp_path) as factory:
monkeypatch.setattr(compat_database, "SessionLocal", factory)
monkeypatch.setattr(resolver, "SessionLocal", factory)
with factory() as db:
db.add(database.ModelEndpoint(id="chat", name="chat", owner="alice",
base_url=CHAT[0], is_enabled=True))
db.add(database.ModelEndpoint(id="shared", name="shared", owner=None,
base_url="https://shared.example/v1", is_enabled=True))
db.commit()
yield
def test_auto_title_inherits_chat_effort_when_utility_is_blank(util, monkeypatch, title_registered_endpoint):
from routes import chat_helpers
from src.endpoint_resolver import build_chat_url
sent = _capture_llm(monkeypatch)
chat_url = build_chat_url(CHAT[0])
asyncio.run(chat_helpers.auto_name_session(_Manager(), _title_session(url=chat_url)))
assert sent == [(chat_url, CHAT[1], "low")]
@pytest.mark.parametrize("owner,url", [
("alice", "https://unregistered.example/v1"),
("bob", CHAT[0]),
(None, "https://shared.example/v1"),
("", "https://shared.example/v1"),
(" ", "https://shared.example/v1"),
(" alice ", "https://shared.example/v1"),
("system", "https://shared.example/v1"),
(42, "https://shared.example/v1"),
])
def test_auto_title_rejects_unauthorized_chat_endpoint_when_utility_is_blank(
util, monkeypatch, title_registered_endpoint, owner, url,
):
from routes import chat_helpers
sent = _capture_llm(monkeypatch)
# Blank utility/task resolves to the session's own route.
monkeypatch.setattr(task_endpoint, "resolve_task_endpoint", lambda *a, **k: (CHAT[0], CHAT[1], {}))
asyncio.run(chat_helpers.auto_name_session(_Manager(), _title_session()))
assert sent == [(CHAT[0], CHAT[1], "low")]
asyncio.run(chat_helpers.auto_name_session(_Manager(), _title_session(owner=owner, url=url)))
assert sent == []
def test_auto_title_missing_owner_fails_closed_in_single_user_mode(util, monkeypatch, title_registered_endpoint):
from routes import chat_helpers
monkeypatch.setenv("AUTH_ENABLED", "false")
sent = _capture_llm(monkeypatch)
asyncio.run(chat_helpers.auto_name_session(
_Manager(), _title_session(owner=None, url="https://shared.example/v1"),
))
assert sent == []
def test_auto_title_setting_beats_chat_effort(util, monkeypatch):