diff --git a/.github/workflows/mailbox-regressions.yml b/.github/workflows/mailbox-regressions.yml new file mode 100644 index 00000000..4b5d8186 --- /dev/null +++ b/.github/workflows/mailbox-regressions.yml @@ -0,0 +1,68 @@ +name: Mailbox regressions +on: + push: + branches: ['fix/mailbox-client-*'] + pull_request: + paths: + - 'src/msg/plugins/mail_views.py' + - 'src/msg/plugins/communication.py' + - 'src/msg/client_subagents_remote.py' + - 'tests/test_mailbox_contacts.py' + - 'tests/test_todo_due_inbox.py' + - 'tests/test_bounty.py' + - 'tests/test_client_subagents_remote.py' + - '.github/workflows/mailbox-regressions.yml' + workflow_dispatch: +permissions: + contents: read +concurrency: + group: mailbox-${{ github.head_ref || github.ref_name }} + cancel-in-progress: true +jobs: + regressions: + runs-on: ubuntu-latest + timeout-minutes: 10 + services: + postgres: + image: postgres:16 + env: + POSTGRES_USER: msgtest + POSTGRES_PASSWORD: msgtest + POSTGRES_DB: postgres + ports: + - 5432:5432 + options: >- + --health-cmd "pg_isready -U msgtest -d postgres" + --health-interval 5s --health-timeout 5s --health-retries 10 + env: + MSG_TEST_POSTGRES_URL_TEMPLATE: postgresql://msgtest:msgtest@127.0.0.1:5432/{database} + steps: + - uses: actions/checkout@v7 + with: + persist-credentials: false + - uses: actions/setup-python@v7 + with: + python-version: '3.15' + allow-prereleases: true + - name: Install locked dependencies + run: | + python -m pip install uv==0.12.20 + uv sync --locked --extra dev --python "$(command -v python)" + - name: Record tested source + run: | + mkdir -p artifacts + git rev-parse HEAD > artifacts/commit.txt + git rev-parse HEAD^{tree} > artifacts/tree.txt + - name: Check system notices, contact privacy and private agent queues + run: >- + uv run --locked --extra dev pytest + tests/test_mailbox_contacts.py tests/test_todo_due_inbox.py + tests/test_bounty.py tests/test_client_subagents_remote.py + tests/test_tui_mailbox_selection.py tests/test_dm.py + --junitxml=artifacts/mailbox.xml + - uses: actions/upload-artifact@v7.0.1 + if: always() + with: + name: mailbox-regressions + path: artifacts/ + if-no-files-found: error diff --git a/src/msg/plugins/mail_views.py b/src/msg/plugins/mail_views.py index c906a73c..ddb159ab 100644 --- a/src/msg/plugins/mail_views.py +++ b/src/msg/plugins/mail_views.py @@ -11,6 +11,9 @@ async def contact(app, ctx, request, tx, subject): from msg.plugins.discovery import visible + # Worker reminders and settlement notices have no sending account. + if subject is None: + return {'name': 'System notification'} if await visible(app, ctx, request, tx, subject): resource = await tx.resource(subject) return {'name': resource.name, 'path': await tx.path(subject)} diff --git a/tests/test_bounty.py b/tests/test_bounty.py index df4e9af6..684a11a7 100644 --- a/tests/test_bounty.py +++ b/tests/test_bounty.py @@ -166,8 +166,11 @@ async def test_prefunded_bounty_pays_offline_publisher_and_stops_at_budget(insta assert await balance(app, publisher_key, publisher) == 10 assert (await call(app, 'money.state', {})).data['total_supply_minor'] == 20 inbox = await call(app, 'communication.inbox', {}, key=claimant_key, subject=claimant) + assert inbox.status == 'ok', wire(inbox) bounty_notices = [item for item in inbox.data['items'] if item.get('source') == 'bounty_claim'] assert len(bounty_notices) == 1 + assert bounty_notices[0]['sender'] is None + assert bounty_notices[0]['sender_contact'] == {'name': 'System notification'} assert bounty_notices[0]['claim_id'] == claimed.data['claim']['id'] replay = await call( app, diff --git a/tests/test_client_subagents_remote.py b/tests/test_client_subagents_remote.py index 8ee4651b..8107c00b 100644 --- a/tests/test_client_subagents_remote.py +++ b/tests/test_client_subagents_remote.py @@ -8,6 +8,7 @@ from msg.client import ClientState, MsgClient from msg.client_subagents_remote import RemoteAgents +from msg.core.codec import b64, wire from msg.core.errors import Failure from msg.transports.client import HTTPTransport from msg.transports.http import create_app @@ -65,6 +66,12 @@ async def test_remote_private_delivery_tail_restart_and_archive(installed, tmp_p await agents.send('bot1', 'bot2', 'after archive') history = await agents.inbox('bot2') assert [x['message'] for x in history['items']] == ['private handoff'] + public = client.checked( + await client.call( + 'file.create', + {'parent': '/main', 'name': 'public-control.txt', 'data': b64(b'Public control')}, + ) + ) for anonymous in (True, False): visitor = client if anonymous else stranger for operation, args in [ @@ -77,11 +84,23 @@ async def test_remote_private_delivery_tail_restart_and_archive(installed, tmp_p result = await visitor.call(operation, args, anonymous=anonymous) assert result.status == 'error' assert 'private handoff' not in repr(result) - visible = await visitor.call( - 'discovery.list', {'type': 'file', 'limit': 200}, anonymous=anonymous - ) - assert visible.status == 'ok' - assert not any('msg-stable-message' in item['name'] for item in visible.data['items']) + # Exercise every page without exhausting one request's read budget. + query = {'type': 'file', 'limit': 20} + cursors, seen = set(), set() + while True: + visible = await visitor.call('discovery.list', query, anonymous=anonymous) + assert visible.status == 'ok', wire(visible.error) + for item in visible.data['items']: + assert 'msg-stable-message' not in item['name'] + assert item['id'] not in seen + seen.add(item['id']) + cursor = visible.data.get('cursor') + if cursor is None: + break + assert cursor not in cursors + cursors.add(cursor) + query['cursor'] = cursor + assert public.resources[0].id in seen feed = await visitor.call('discovery.recommendations', {}, anonymous=anonymous) assert 'private handoff' not in repr(feed) users = client.checked( diff --git a/tests/test_mailbox_contacts.py b/tests/test_mailbox_contacts.py new file mode 100644 index 00000000..eaa4c838 --- /dev/null +++ b/tests/test_mailbox_contacts.py @@ -0,0 +1,80 @@ +"""System notices remain readable without bypassing real contact privacy.""" + +from dataclasses import replace +from datetime import timedelta + +import pytest +from read_only_evidence import readonly_evidence +from test_oauth import browser_login, oauth as oauth +from test_service import call, register + +from msg.core.codec import wire +from msg.workers.maintenance import run_maintenance + + +@pytest.mark.asyncio +async def test_browser_system_notice_has_no_fake_user_and_reads_have_no_effects(oauth, monkeypatch): + app, key, subject, http = oauth + todo = await call( + app, + 'identity.todo_put', + { + 'name': 'browser-due', + 'title': 'Private browser reminder', + 'due_at': wire(app.clock() - timedelta(hours=1)), + }, + key=key, + subject=subject, + ) + assert todo.status == 'ok', wire(todo) + assert await run_maintenance(app, 'deliver_due_todos', scheduled=True) == { + 'delivered_todo_reminders': 1 + } + assert (await http.get('/@oauth-owner/in')).status_code == 401 + await browser_login(oauth) + async with readonly_evidence(app, monkeypatch): + for accept in ('text/html', 'text/markdown'): + response = await http.get('/@oauth-owner/in', headers={'Accept': accept}) + assert response.status_code == 200, response.text + assert 'System notification' in response.text + assert '/@None' not in response.text + + +@pytest.mark.asyncio +@pytest.mark.parametrize('mailbox', ['inbox', 'outbox']) +async def test_real_private_contact_still_hides_name_and_path(installed, monkeypatch, mailbox): + app, _ = installed + author_key, author, _ = await register(app, 'contact-author') + recipient_key, recipient, _ = await register(app, 'contact-recipient') + post = await call( + app, + 'content.post_create', + {'parent': '/main', 'body': '# Reference\n\nReadable reference with a private contact'}, + key=author_key, + subject=author, + ) + assert post.status == 'ok', wire(post) + sent = await call( + app, + 'communication.send', + {'recipient': recipient, 'resource': {'id': post.resources[0].id}}, + key=author_key, + subject=author, + ) + assert sent.status == 'ok', wire(sent) + key, subject, hidden, label = ( + (recipient_key, recipient, author, 'sender_contact') + if mailbox == 'inbox' + else (author_key, author, recipient, 'recipient_contact') + ) + async with app.metadata.transaction(write=True) as tx: + resource = await tx.resource(hidden) + await tx.replace( + replace(resource, mode=0o600, generation=resource.generation + 1), resource.generation + ) + async with readonly_evidence(app, monkeypatch): + result = await call(app, 'communication.' + mailbox, {}, key=key, subject=subject) + assert result.status == 'ok', wire(result) + item = next(item for item in result.data['items'] if item['id'] == sent.data['message_id']) + assert item[label] == {'name': 'Private account'} + assert item['preview']['excerpt'] == 'Readable reference with a private contact' diff --git a/tests/test_todo_due_inbox.py b/tests/test_todo_due_inbox.py index 8df87e36..f62843a6 100644 --- a/tests/test_todo_due_inbox.py +++ b/tests/test_todo_due_inbox.py @@ -47,9 +47,13 @@ async def test_due_todo_is_only_in_owner_inbox_and_repeated_maintenance_is_noop( assert tx.one('SELECT COUNT(*) FROM audit')[0] == before + 1 assert tx.one('SELECT COUNT(*) FROM messages WHERE resource=?', (rid,))[0] == 1 own = await call(app, 'communication.inbox', {}, key=owner_key, subject=owner) + assert own.status == 'ok', wire(own) notices = [item for item in own.data['items'] if item['resource']['id'] == rid] assert len(notices) == 1 and notices[0]['source'] == 'todo_due' assert notices[0]['recipient'] == owner + assert notices[0]['sender'] is None + assert notices[0]['sender_contact'] == {'name': 'System notification'} + assert notices[0]['recipient_contact'] == {'name': '@due-owner', 'path': '/@due-owner'} assert notices[0]['todo_revision_at_delivery'] == todo.resources[0].revision assert not [ item