diff --git a/mobile/README.md b/mobile/README.md index c93dacc636..5cda9e0e1e 100644 --- a/mobile/README.md +++ b/mobile/README.md @@ -51,11 +51,14 @@ any length while the QR stays small. } ``` -- `v` (1 or 2), `id` (the link's card id), `name` and `avatarUrl` are required. - `instructions`, `model` and `reasoningEffort` are optional and left for the - user when absent; `reasoningEffort` needs `model`. A missing required field, - a known field of the wrong type, or an avatar that isn't an `https` URL on the - same CDN makes the card invalid, and no form opens. Other keys are ignored. +- `id` (the link's card id), `name` and `avatarUrl` (an `https` URL) are + required. `instructions`, `model` and `reasoningEffort` are optional and left + for the user when absent. A missing required field or a known field of the + wrong type makes the card invalid, and no form opens. Other keys, including + `v`, are ignored. The card id is opaque to the app. +- The fetch shares the image loader's limits (`lib/shared/utils/bounded_fetch.dart`): + it is aborted after 20 seconds without a byte or 5 minutes overall, and a card + over 2 MB is rejected. - `model` and `reasoningEffort` are pre-selected only if the live model catalog offers them (the effort only for that model). - The link waits for sign-in and community load. It needs Builderlab login and a diff --git a/mobile/lib/features/agents/agent_creation_draft.dart b/mobile/lib/features/agents/agent_creation_draft.dart index 5b628b8453..4fb5c27b65 100644 --- a/mobile/lib/features/agents/agent_creation_draft.dart +++ b/mobile/lib/features/agents/agent_creation_draft.dart @@ -1,12 +1,11 @@ -import 'dart:async'; import 'dart:convert'; -import 'dart:typed_data'; import 'package:hooks_riverpod/hooks_riverpod.dart'; import 'package:http/http.dart' as http; import '../../shared/deeplink/deep_link.dart'; import '../../shared/hosted/hosted_environment.dart'; +import '../../shared/utils/bounded_fetch.dart'; import 'agent_avatar_picker.dart'; /// Creates the HTTP client that fetches a card's `agent.json`; overridden in @@ -16,9 +15,7 @@ final agentCardClientProvider = Provider( ); /// Largest `agent.json` accepted, far above any real card. -const maxAgentCardBytes = 256 * 1024; - -const _fetchTimeout = Duration(seconds: 15); +const maxAgentCardBytes = 2 * 1024 * 1024; /// Why a card's draft could not be loaded, worded for the user. class AgentCardException implements Exception { @@ -61,64 +58,56 @@ class AgentCreationDraft { http.Client client, ) async { final cdn = HostedEnvironment.avatarsCdnOrigin; - final url = cdn.replace(path: '/${link.cardId}/agent.json'); - final http.StreamedResponse response; - final List body; + // The id is opaque; pathSegments percent-encodes anything path-unsafe. + final url = cdn.replace(pathSegments: [link.cardId, 'agent.json']); + final BoundedResponse response; try { - response = await client - .send( - http.Request('GET', url) - ..headers['accept'] = 'application/json' - // A redirect could lead off the CDN; treat it as unavailable. - ..followRedirects = false, - ) - .timeout(_fetchTimeout); - if (response.statusCode == 403 || response.statusCode == 404) { - throw const AgentCardException('This agent card isn’t available.'); - } - if (response.statusCode != 200) { - throw AgentCardException( - 'Couldn’t load this agent card (HTTP ${response.statusCode}).', - ); - } - if ((response.contentLength ?? 0) > maxAgentCardBytes) { - throw const AgentCardException('This agent card is too large.'); - } - body = await _readBounded(response.stream).timeout(_fetchTimeout); - } on AgentCardException { - rethrow; + response = await boundedGet( + client, + url, + maxBytes: maxAgentCardBytes, + headers: const {'accept': 'application/json'}, + // A redirect could lead off the CDN; treat it as unavailable. + followRedirects: false, + ); + } on ResponseTooLargeException { + throw const AgentCardException('This agent card is too large.'); } on Exception { throw const AgentCardException( 'Couldn’t load this agent card. Check your connection and reopen the ' 'link.', ); } + if (response.statusCode == 403 || response.statusCode == 404) { + throw const AgentCardException('This agent card isn’t available.'); + } + if (response.statusCode != 200) { + throw AgentCardException( + 'Couldn’t load this agent card (HTTP ${response.statusCode}).', + ); + } final Object? json; try { - json = jsonDecode(utf8.decode(body)); + json = jsonDecode(utf8.decode(response.bytes)); } on FormatException { throw const AgentCardException('This agent card is invalid.'); } if (json is! Map) { throw const AgentCardException('This agent card is invalid.'); } - return parseAgentCard(json, link.cardId, cdn); + return parseAgentCard(json, link.cardId); } - /// Reads a card's draft from its `agent.json`, validated against the card - /// schema (v1 or v2). `v`, `id` (matching [cardId]), `name` and `avatarUrl` - /// are required; `instructions`, `model` and `reasoningEffort` are optional - /// and left for the user when absent. A required field that is missing, or - /// any known field of the wrong type, makes the card invalid. Other keys are - /// ignored. The avatar must be an https URL on [cdn], the card's own host. + /// Reads a card's draft from its `agent.json`. `id` (matching [cardId]), + /// `name` and `avatarUrl` (an https URL) are required; `instructions`, + /// `model` and `reasoningEffort` are optional and left for the user when + /// absent or blank. A missing required field, or a known field of the wrong + /// type, makes the card invalid. Other keys, including `v`, are ignored. static AgentCreationDraft parseAgentCard( Map json, String cardId, - Uri cdn, ) { const invalid = AgentCardException('This agent card is invalid.'); - final version = json['v']; - if (version is! int || (version != 1 && version != 2)) throw invalid; if (json['id'] != cardId) throw invalid; final name = json['name']; final avatarUrl = json['avatarUrl']; @@ -126,11 +115,7 @@ class AgentCreationDraft { throw invalid; } final avatar = Uri.tryParse(avatarUrl); - if (avatar == null || - avatar.scheme != 'https' || - avatar.host != cdn.host || - avatar.hasPort || - avatar.userInfo.isNotEmpty) { + if (avatar == null || avatar.scheme != 'https' || avatar.host.isEmpty) { throw invalid; } String? optional(String key) { @@ -140,26 +125,12 @@ class AgentCreationDraft { return text == null || text.isEmpty ? null : text; } - final modelId = optional('model'); - final reasoningEffort = optional('reasoningEffort'); - if (reasoningEffort != null && modelId == null) throw invalid; return AgentCreationDraft( name: name.trim(), instructions: optional('instructions') ?? '', avatar: AgentAvatarSelection.url(avatarUrl), - modelId: modelId, - reasoningEffort: reasoningEffort, + modelId: optional('model'), + reasoningEffort: optional('reasoningEffort'), ); } - - static Future> _readBounded(Stream> stream) async { - final bytes = BytesBuilder(copy: false); - await for (final chunk in stream) { - if (bytes.length + chunk.length > maxAgentCardBytes) { - throw const AgentCardException('This agent card is too large.'); - } - bytes.add(chunk); - } - return bytes.takeBytes(); - } } diff --git a/mobile/lib/shared/deeplink/agent_creation_link.dart b/mobile/lib/shared/deeplink/agent_creation_link.dart index 5636ce48c2..ea4f78ef61 100644 --- a/mobile/lib/shared/deeplink/agent_creation_link.dart +++ b/mobile/lib/shared/deeplink/agent_creation_link.dart @@ -11,7 +11,7 @@ class AgentCreationDeepLink extends BuzzDeepLink { /// Creates a link for the published card [cardId]. const AgentCreationDeepLink(this.cardId); - /// Published card id, a plain URL-safe token such as `av_…`. + /// Published card id, opaque to the app (today `av_…`). final String cardId; @override @@ -25,8 +25,6 @@ class AgentCreationDeepLink extends BuzzDeepLink { String toString() => 'AgentCreationDeepLink($cardId)'; } -final _cardId = RegExp(r'^[A-Za-z0-9_-]{1,64}$'); - /// Parses `buzz://agent/new?card=`; anything else is not this link. AgentCreationDeepLink? parseAgentCreationDeepLink(Uri uri) { if (uri.scheme != 'buzz' || @@ -40,6 +38,7 @@ AgentCreationDeepLink? parseAgentCreationDeepLink(Uri uri) { final params = uri.queryParametersAll; final card = params['card']; if (params.length != 1 || card == null || card.length != 1) return null; - if (!_cardId.hasMatch(card.single)) return null; + // The id is opaque, but it becomes one path segment of the card's URL. + if (const {'', '.', '..'}.contains(card.single)) return null; return AgentCreationDeepLink(card.single); } diff --git a/mobile/lib/shared/relay/media_image.dart b/mobile/lib/shared/relay/media_image.dart index 7d8f016fdb..e46381b389 100644 --- a/mobile/lib/shared/relay/media_image.dart +++ b/mobile/lib/shared/relay/media_image.dart @@ -1,5 +1,4 @@ import 'dart:async'; -import 'dart:typed_data'; import 'dart:ui' as ui; import 'package:flutter/foundation.dart'; @@ -9,6 +8,7 @@ import 'package:hooks_riverpod/hooks_riverpod.dart'; import 'package:http/http.dart' as http; import '../hosted/hosted_relay_auth.dart'; +import '../utils/bounded_fetch.dart'; import 'media_auth.dart'; /// Shared, keep-alive HTTP client for media fetches. One client per container @@ -71,8 +71,8 @@ class MediaImageProvider extends ImageProvider { this.onBytesLoaded, this.isThumbnailFallback = false, this.scale = 1.0, - this.stallTimeout = const Duration(seconds: 20), - this.maxFetchDuration = const Duration(minutes: 5), + this.stallTimeout = defaultStallTimeout, + this.maxFetchDuration = defaultMaxFetchDuration, }); /// Maximum encoded image body retained before decoding (20 MiB). @@ -185,63 +185,19 @@ class MediaImageProvider extends ImageProvider { } final uri = Uri.parse(url); - final http.Response response; - var wasAbandoned = false; + final BoundedResponse response; try { - final stop = Completer(); - Timer? stall; - void restartStallTimer() { - stall?.cancel(); - stall = Timer(stallTimeout, () { - if (!stop.isCompleted) stop.complete(); - }); - } - - restartStallTimer(); - final deadline = Timer(maxFetchDuration, () { - if (!stop.isCompleted) stop.complete(); - }); - unawaited( - abort.then((_) { - if (stop.isCompleted) return; - wasAbandoned = true; - stop.complete(); - }), + response = await boundedGet( + client, + uri, + maxBytes: maxResponseBytes, + headers: auth.headersFor(url), + stallTimeout: stallTimeout, + maxFetchDuration: maxFetchDuration, + cancel: abort, ); - StreamIterator>? body; - try { - final request = http.AbortableRequest( - 'GET', - uri, - abortTrigger: stop.future, - )..headers.addAll(auth.headersFor(url)); - final streamed = await client.send(request); - restartStallTimer(); - final iterator = body = StreamIterator(streamed.stream); - final bytes = BytesBuilder(copy: false); - // An error body is never shown, and reading a slow one could hold - // this load for the whole fetch cap before the fallback or - // Retry-After applies. Leave it unread; finally cancels the stream. - while (streamed.statusCode == 200 && await iterator.moveNext()) { - restartStallTimer(); - final chunk = iterator.current; - if (chunk.length > maxResponseBytes - bytes.length) { - throw const FormatException('Image response too large'); - } - bytes.add(chunk); - } - response = http.Response.bytes( - bytes.takeBytes(), - streamed.statusCode, - headers: streamed.headers, - ); - } finally { - stall?.cancel(); - deadline.cancel(); - if (!stop.isCompleted) stop.complete(); - unawaited(body?.cancel()); - } - } catch (_) { + } catch (error) { + final wasAbandoned = error is FetchCancelledException; // Nobody wanted the image any more; the URL itself did not fail. if (!wasAbandoned) { _cooldownUntil[url] = debugNow().add(_defaultCooldown); @@ -263,7 +219,7 @@ class MediaImageProvider extends ImageProvider { uri: uri, ); } - final bytes = response.bodyBytes; + final bytes = response.bytes; if (bytes.isEmpty) { _cooldownUntil[url] = debugNow().add(_defaultCooldown); throw NetworkImageLoadException(statusCode: 200, uri: uri); @@ -271,7 +227,7 @@ class MediaImageProvider extends ImageProvider { return bytes; } - Duration _cooldownFor(http.Response response) { + Duration _cooldownFor(BoundedResponse response) { final retryAfter = int.tryParse(response.headers['retry-after'] ?? ''); if (retryAfter != null && retryAfter > 0) { final requested = Duration(seconds: retryAfter); diff --git a/mobile/lib/shared/utils/bounded_fetch.dart b/mobile/lib/shared/utils/bounded_fetch.dart new file mode 100644 index 0000000000..eead71b407 --- /dev/null +++ b/mobile/lib/shared/utils/bounded_fetch.dart @@ -0,0 +1,120 @@ +import 'dart:async'; +import 'dart:typed_data'; + +import 'package:http/http.dart' as http; + +/// Longest wait for response headers or the next body chunk. A slow link that +/// keeps delivering bytes finishes; one that stops is aborted. +const defaultStallTimeout = Duration(seconds: 20); + +/// Longest a fetch may take even while bytes keep arriving, so a server that +/// trickles bytes cannot hold a connection indefinitely. +const defaultMaxFetchDuration = Duration(minutes: 5); + +/// A GET response read by [boundedGet]. [bytes] is empty unless the status +/// was 200: an error body is never read. +class BoundedResponse { + const BoundedResponse(this.statusCode, this.headers, this.bytes); + + final int statusCode; + final Map headers; + final Uint8List bytes; +} + +/// The 200 body of a [boundedGet] exceeded its `maxBytes`. +class ResponseTooLargeException implements Exception { + const ResponseTooLargeException(this.uri); + + final Uri uri; + + @override + String toString() => 'ResponseTooLargeException: $uri'; +} + +/// The caller's `cancel` future completed before [boundedGet] finished. +class FetchCancelledException implements Exception { + const FetchCancelledException(this.uri); + + final Uri uri; + + @override + String toString() => 'FetchCancelledException: $uri'; +} + +/// GETs [uri] without closing [client], aborting the request when no headers +/// or body bytes arrive for [stallTimeout], when it runs past +/// [maxFetchDuration], or when [cancel] completes. A 200 body larger than +/// [maxBytes], declared or streamed, throws [ResponseTooLargeException]; other statuses return +/// without reading the body. A stall or deadline throws the client's abort +/// error ([http.RequestAbortedException]); a cancel throws +/// [FetchCancelledException]. +Future boundedGet( + http.Client client, + Uri uri, { + required int maxBytes, + Map headers = const {}, + bool followRedirects = true, + Duration stallTimeout = defaultStallTimeout, + Duration maxFetchDuration = defaultMaxFetchDuration, + Future? cancel, +}) async { + final stop = Completer(); + var cancelled = false; + Timer? stall; + void restartStallTimer() { + stall?.cancel(); + stall = Timer(stallTimeout, () { + if (!stop.isCompleted) stop.complete(); + }); + } + + restartStallTimer(); + final deadline = Timer(maxFetchDuration, () { + if (!stop.isCompleted) stop.complete(); + }); + unawaited( + cancel?.then((_) { + if (stop.isCompleted) return; + cancelled = true; + stop.complete(); + }), + ); + StreamIterator>? body; + try { + final request = http.AbortableRequest('GET', uri, abortTrigger: stop.future) + ..headers.addAll(headers) + ..followRedirects = followRedirects; + final streamed = await client.send(request); + restartStallTimer(); + if (streamed.statusCode == 200 && + (streamed.contentLength ?? 0) > maxBytes) { + unawaited(streamed.stream.listen(null).cancel()); + throw ResponseTooLargeException(uri); + } + final iterator = body = StreamIterator(streamed.stream); + final bytes = BytesBuilder(copy: false); + // An error body is never shown, and reading a slow one could hold the + // caller for the whole fetch cap. Leave it unread; finally cancels it. + while (streamed.statusCode == 200 && await iterator.moveNext()) { + restartStallTimer(); + final chunk = iterator.current; + if (chunk.length > maxBytes - bytes.length) { + throw ResponseTooLargeException(uri); + } + bytes.add(chunk); + } + return BoundedResponse( + streamed.statusCode, + streamed.headers, + bytes.takeBytes(), + ); + } catch (_) { + if (cancelled) throw FetchCancelledException(uri); + rethrow; + } finally { + stall?.cancel(); + deadline.cancel(); + if (!stop.isCompleted) stop.complete(); + unawaited(body?.cancel()); + } +} diff --git a/mobile/test/shared/deeplink/agent_creation_link_test.dart b/mobile/test/shared/deeplink/agent_creation_link_test.dart index 93121f43c9..3ab35e57d0 100644 --- a/mobile/test/shared/deeplink/agent_creation_link_test.dart +++ b/mobile/test/shared/deeplink/agent_creation_link_test.dart @@ -14,9 +14,9 @@ void main() { 'buzz://agent/new?card=av_MioTAG6dm7q3Zcm0UV2j8A', 'av_MioTAG6dm7q3Zcm0UV2j8A', ), - 'any URL-safe token': ( - 'buzz://agent/new?card=abc-DEF_123', - 'abc-DEF_123', + 'any opaque id': ( + 'buzz://agent/new?card=card.7~x%2Fy%20z', + 'card.7~x/y z', ), }; accepted.forEach((label, c) { @@ -28,10 +28,7 @@ void main() { const rejected = { 'no card': 'buzz://agent/new', 'empty card': 'buzz://agent/new?card=', - 'path in card': 'buzz://agent/new?card=..%2Fx', - 'host in card': 'buzz://agent/new?card=evil.example%2Fx', - 'card too long': - 'buzz://agent/new?card=aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa', + 'dot segment': 'buzz://agent/new?card=..', 'two cards': 'buzz://agent/new?card=a&card=b', 'extra parameter': 'buzz://agent/new?card=a&auto=true', 'inline draft (retired format)': 'buzz://agent/new#e30', @@ -51,7 +48,6 @@ void main() { }); group('agent.json', () { - final cdn = Uri.parse('https://cdn.example'); const avatar = 'https://cdn.example/av_1/rev/avatar.png'; // A card as kgoose publishes it today (schema v1). const published = { @@ -65,8 +61,8 @@ void main() { 'shareUrl': 'https://app.builderlab.xyz/a/av_1', }; - test('a published v1 card opens with name and avatar only', () { - final draft = AgentCreationDraft.parseAgentCard(published, 'av_1', cdn); + test('a published card opens with name and avatar only', () { + final draft = AgentCreationDraft.parseAgentCard(published, 'av_1'); expect(draft.name, 'Nitrox'); expect(draft.instructions, ''); expect(draft.avatar?.url, avatar); @@ -74,75 +70,61 @@ void main() { expect(draft.reasoningEffort, isNull); }); - test('a v2 card carries instructions and model defaults', () { - final draft = AgentCreationDraft.parseAgentCard( - { - ...published, - 'v': 2, - 'name': ' Scout ', - 'instructions': 'Reply concisely.', - 'model': 'sonnet-5-5', - 'reasoningEffort': 'high', - // Other keys are ignored, including other spellings of known ones. - 'system_prompt': 'ignored', - 'model_id': 'ignored', - }, - 'av_1', - cdn, - ); + test('a card carries instructions and model defaults', () { + final draft = AgentCreationDraft.parseAgentCard({ + ...published, + 'v': 99, + 'name': ' Scout ', + 'avatarUrl': 'https://images.example/scout.png', + 'instructions': 'Reply concisely.', + 'model': 'sonnet-5-5', + 'reasoningEffort': 'high', + // Other keys are ignored, including other spellings of known ones. + 'system_prompt': 'ignored', + 'model_id': 'ignored', + }, 'av_1'); expect(draft.name, 'Scout'); + expect(draft.avatar?.url, 'https://images.example/scout.png'); expect(draft.instructions, 'Reply concisely.'); expect(draft.modelId, 'sonnet-5-5'); expect(draft.reasoningEffort, 'high'); }); test('blank optional fields are left for the user', () { - final draft = AgentCreationDraft.parseAgentCard( - {...published, 'v': 2, 'instructions': '', 'model': ' '}, - 'av_1', - cdn, - ); + final draft = AgentCreationDraft.parseAgentCard({ + ...published, + 'instructions': '', + 'model': ' ', + }, 'av_1'); expect(draft.instructions, ''); expect(draft.modelId, isNull); }); final invalid = >{ - 'missing v': {...published}..remove('v'), - 'unknown v': {...published, 'v': 3}, - 'string v': {...published, 'v': '1'}, 'missing id': {...published}..remove('id'), 'id of another card': {...published, 'id': 'av_2'}, 'missing name': {...published}..remove('name'), 'blank name': {...published, 'name': ' '}, 'non-string name': {...published, 'name': 7}, 'missing avatar': {...published}..remove('avatarUrl'), - 'avatar on another host': { - ...published, - 'avatarUrl': 'https://evil.example/a.png', - }, + 'non-string avatar': {...published, 'avatarUrl': 7}, 'http avatar': {...published, 'avatarUrl': 'http://cdn.example/a.png'}, - 'avatar with credentials': { - ...published, - 'avatarUrl': 'https://u:p@cdn.example/a.png', - }, + 'relative avatar': {...published, 'avatarUrl': '/av_1/a.png'}, 'non-string instructions': { ...published, - 'v': 2, 'instructions': ['x'], }, - 'non-string model': {...published, 'v': 2, 'model': 5}, + 'non-string model': {...published, 'model': 5}, 'non-string effort': { ...published, - 'v': 2, 'model': 'sonnet-5-5', 'reasoningEffort': true, }, - 'effort without model': {...published, 'v': 2, 'reasoningEffort': 'high'}, }; invalid.forEach((label, json) { test(label, () { expect( - () => AgentCreationDraft.parseAgentCard(json, 'av_1', cdn), + () => AgentCreationDraft.parseAgentCard(json, 'av_1'), throwsA( isA().having( (e) => e.message, @@ -205,6 +187,18 @@ void main() { expect(seen.followRedirects, isFalse); }); + test('percent-encodes the opaque id as one path segment', () async { + late Uri seen; + await AgentCreationDraft.fetch( + const AgentCreationDeepLink('a/b c'), + MockClient.streaming((request, _) async { + seen = request.url; + return respond(404); + }), + ).then((_) {}, onError: (_) {}); + expect(seen.path, '/a%2Fb%20c/agent.json'); + }); + final failures = { '403 from S3': ( (_, _) async => respond(403), diff --git a/mobile/test/shared/relay/media_image_test.dart b/mobile/test/shared/relay/media_image_test.dart index 291139f844..91dbb7d912 100644 --- a/mobile/test/shared/relay/media_image_test.dart +++ b/mobile/test/shared/relay/media_image_test.dart @@ -4,6 +4,7 @@ import 'dart:typed_data'; import 'package:buzz/shared/relay/media_auth.dart'; import 'package:buzz/shared/relay/media_image.dart'; +import 'package:buzz/shared/utils/bounded_fetch.dart'; import 'package:flutter/material.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:hooks_riverpod/hooks_riverpod.dart'; @@ -177,7 +178,7 @@ void main() { final error = await _waitError( provider.resolve(ImageConfiguration.empty), ).timeout(const Duration(seconds: 2)); - expect(error, isA()); + expect(error, isA()); expect(cancelled, isTrue); expect(aborted, isTrue); expect(delivered, isFalse);