From 3e10c2d3468ac77fa7a735a1d050573a7fbf1793 Mon Sep 17 00:00:00 2001 From: Rob Becker Date: Tue, 15 Sep 2026 22:37:23 -0600 Subject: [PATCH] feat(transport): introduce typed HTTP transport and enforce security boundaries Co-authored-by: Cursor --- lib/src/browser/xplat_browser.dart | 17 +- lib/src/common.dart | 5 + lib/src/common/github.dart | 202 ++++----- lib/src/common/transport/error_decoder.dart | 233 ++++++++++ lib/src/common/transport/request.dart | 46 ++ lib/src/common/transport/retry_policy.dart | 148 +++++++ lib/src/common/transport/transport.dart | 280 ++++++++++++ lib/src/common/url_shortener_service.dart | 24 +- lib/src/common/util/errors.dart | 195 ++++++++- lib/src/server/hooks.dart | 216 +++++++++- lib/src/server/xplat_server.dart | 3 +- test/unit/security_test.dart | 305 ++++++++++++++ test/unit/transport_test.dart | 445 ++++++++++++++++++++ 13 files changed, 1937 insertions(+), 182 deletions(-) create mode 100644 lib/src/common/transport/error_decoder.dart create mode 100644 lib/src/common/transport/request.dart create mode 100644 lib/src/common/transport/retry_policy.dart create mode 100644 lib/src/common/transport/transport.dart create mode 100644 test/unit/security_test.dart create mode 100644 test/unit/transport_test.dart diff --git a/lib/src/browser/xplat_browser.dart b/lib/src/browser/xplat_browser.dart index c54a2530..78f4ba13 100644 --- a/lib/src/browser/xplat_browser.dart +++ b/lib/src/browser/xplat_browser.dart @@ -5,13 +5,18 @@ import 'package:github/src/common.dart'; import 'package:github/src/common/xplat_common.dart' show findAuthenticationInMap; -/// Looks for GitHub Authentication information from the browser +/// Looks for GitHub Authentication information from the browser. /// -/// Checks for query strings first, then local storage using keys in [COMMON_GITHUB_TOKEN_ENV_KEYS]. -/// If the above fails, the GITHUB_USERNAME and GITHUB_PASSWORD keys will be checked. -Authentication findAuthenticationFromEnvironment() { - // search the query string parameters first - var auth = findAuthenticationInMap(_parseQuery(window.location.href)); +/// NOTE: Passing credentials in query strings is insecure as tokens can be +/// leaked via logs, browser history, screenshots, and Referer headers. +/// Extracting tokens from URL query parameters is disabled by default and deprecated. +/// Explicitly instantiate [Authentication] or use `window.sessionStorage`. +Authentication findAuthenticationFromEnvironment( + {bool allowQueryAuth = false}) { + Authentication? auth; + if (allowQueryAuth) { + auth = findAuthenticationInMap(_parseQuery(window.location.href)); + } auth ??= findAuthenticationInMap(window.sessionStorage); return auth ?? const Authentication.anonymous(); } diff --git a/lib/src/common.dart b/lib/src/common.dart index df783a26..e9874934 100644 --- a/lib/src/common.dart +++ b/lib/src/common.dart @@ -5,6 +5,7 @@ library; export 'package:github/src/common/activity_service.dart'; export 'package:github/src/common/authorizations_service.dart'; export 'package:github/src/common/checks_service.dart'; +export 'package:github/src/common/generated/rest_contracts.g.dart'; export 'package:github/src/common/gists_service.dart'; export 'package:github/src/common/git_service.dart'; export 'package:github/src/common/github.dart'; @@ -40,6 +41,10 @@ export 'package:github/src/common/orgs_service.dart'; export 'package:github/src/common/pulls_service.dart'; export 'package:github/src/common/repos_service.dart'; export 'package:github/src/common/search_service.dart'; +export 'package:github/src/common/transport/error_decoder.dart'; +export 'package:github/src/common/transport/request.dart'; +export 'package:github/src/common/transport/retry_policy.dart'; +export 'package:github/src/common/transport/transport.dart'; export 'package:github/src/common/url_shortener_service.dart'; export 'package:github/src/common/users_service.dart'; export 'package:github/src/common/util/auth.dart'; diff --git a/lib/src/common/github.dart b/lib/src/common/github.dart index e6ba64cb..807daa41 100644 --- a/lib/src/common/github.dart +++ b/lib/src/common/github.dart @@ -23,7 +23,48 @@ class GitHub { this.endpoint = 'https://api.github.com', this.version = '2022-11-28', http.Client? client, - }) : client = client ?? http.Client(); + Set? trustedOrigins, + this.allowInsecureAuth = false, + this.retryPolicy = const RetryPolicy(), + HttpTransport? transport, + }) : client = client ?? http.Client(), + trustedOrigins = trustedOrigins ?? _defaultTrustedOrigins(endpoint), + _transport = transport; + + static Set _defaultTrustedOrigins(String endpoint) { + final origins = {}; + final uri = Uri.tryParse(endpoint); + if (uri != null && uri.hasScheme && uri.hasAuthority) { + origins.add(uri.origin); + } else { + origins.add('https://api.github.com'); + } + origins.add('https://uploads.github.com'); + return origins; + } + + /// Policy governing retries, exponential backoff, and rate limits. + final RetryPolicy retryPolicy; + + /// Underlying injectable HTTP transport. + HttpTransport get transport => _transport ??= HttpTransport( + github: this, + client: client, + retryPolicy: retryPolicy, + trustedOrigins: trustedOrigins, + allowInsecureAuth: allowInsecureAuth, + endpoint: endpoint, + ); + HttpTransport? _transport; + + /// Set of trusted origins permitted to receive authentication credentials. + /// Defaults to the origin of [endpoint] (e.g. `https://api.github.com`) and + /// `https://uploads.github.com`. + final Set trustedOrigins; + + /// Whether to allow sending credentials over unencrypted HTTP. + /// Defaults to `false` for security. Set to `true` only for local test servers. + final bool allowInsecureAuth; static const _ratelimitLimitHeader = 'x-ratelimit-limit'; static const _ratelimitResetHeader = 'x-ratelimit-reset'; @@ -51,6 +92,7 @@ class GitHub { final http.Client client; ActivityService? _activity; + // ignore: deprecated_member_use_from_same_package AuthorizationsService? _authorizations; GistsService? _gists; GitService? _git; @@ -60,6 +102,7 @@ class GitHub { PullRequestsService? _pullRequests; RepositoriesService? _repositories; SearchService? _search; + // ignore: deprecated_member_use_from_same_package UrlShortenerService? _urlShortener; UsersService? _users; ChecksService? _checks; @@ -98,7 +141,9 @@ class GitHub { /// /// Note: You can only access this API via Basic Authentication using your /// username and password, not tokens. + // ignore: deprecated_member_use_from_same_package AuthorizationsService get authorizations => + // ignore: deprecated_member_use_from_same_package _authorizations ??= AuthorizationsService(this); /// Service for gist related methods of the GitHub API. @@ -129,7 +174,9 @@ class GitHub { SearchService get search => _search ??= SearchService(this); /// Service to provide a handy method to access GitHub's url shortener. + // ignore: deprecated_member_use_from_same_package UrlShortenerService get urlShortener => + // ignore: deprecated_member_use_from_same_package _urlShortener ??= UrlShortenerService(this); /// Service for user related methods of the GitHub API. @@ -330,10 +377,16 @@ class GitHub { fail: fail, ); + if (response.statusCode == 204 || response.body.isEmpty) { + return null as T; + } + final json = jsonDecode(response.body); - final returnValue = convert(json) as T; - _applyExpandos(returnValue, response); + final returnValue = convert(json as S) as T; + if (returnValue != null) { + _applyExpandos(returnValue, response); + } return returnValue; } @@ -355,135 +408,27 @@ class GitHub { void Function(http.Response response)? fail, String? preview, }) async { - if (rateLimitRemaining != null && rateLimitRemaining! <= 0) { - assert(rateLimitReset != null); - final now = DateTime.now(); - final waitTime = rateLimitReset!.difference(now); - await Future.delayed(waitTime); - } - headers ??= {}; if (preview != null) { headers['Accept'] = preview; } - final authHeaderValue = auth.authorizationHeaderValue(); - if (authHeaderValue != null) { - headers.putIfAbsent('Authorization', () => authHeaderValue); - } - - // See https://docs.github.com/en/rest/overview/resources-in-the-rest-api?apiVersion=2022-11-28#user-agent-required - headers.putIfAbsent('User-Agent', () => auth.username ?? 'github.dart'); - - if (method == 'PUT' && body == null) { - headers.putIfAbsent('Content-Length', () => '0'); - } - - var queryString = ''; - - if (params != null) { - queryString = buildQueryString(params); - } - - final url = StringBuffer(); - - if (path.startsWith('http://') || path.startsWith('https://')) { - url.write(path); - url.write(queryString); - } else { - url.write(endpoint); - if (!path.startsWith('/')) { - url.write('/'); - } - url.write(path); - url.write(queryString); - } - - final request = http.Request(method, Uri.parse(url.toString())); - request.headers.addAll(headers); - if (body != null) { - if (body is List) { - request.bodyBytes = body; - } else { - request.body = body.toString(); - } - } - - final streamedResponse = await client.send(request); - - final response = await http.Response.fromStream(streamedResponse); + final apiRequest = ApiRequest( + method: method, + path: path, + headers: headers, + params: params ?? const {}, + body: body, + successStatuses: statusCode != null ? {statusCode} : null, + ); - _updateRateLimit(response.headers); - if (statusCode != null && statusCode != response.statusCode) { - if (fail != null) { - fail(response); - } - handleStatusCode(response); - } else { - return response; - } + return transport.execute(apiRequest, fail: fail); } - /// /// Internal method to handle status codes - /// Never handleStatusCode(http.Response response) { - String? message = ''; - List>? errors; - if (response.headers['content-type']!.contains('application/json')) { - try { - final json = jsonDecode(response.body); - message = json['message']; - if (json['errors'] != null) { - try { - errors = List>.from(json['errors']); - } catch (_) { - errors = [ - {'code': json['errors'].toString()} - ]; - } - } - } catch (ex) { - throw UnknownError(this, ex.toString()); - } - } - switch (response.statusCode) { - case 404: - throw NotFound(this, 'Requested Resource was Not Found'); - case 401: - throw AccessForbidden(this); - case 400: - if (message == 'Problems parsing JSON') { - throw InvalidJSON(this, message); - } else if (message == 'Body should be a JSON Hash') { - throw InvalidJSON(this, message); - } else { - throw BadRequest(this); - } - case 422: - final buff = StringBuffer(); - buff.writeln(); - buff.writeln(' Message: $message'); - if (errors != null) { - buff.writeln(' Errors:'); - for (final error in errors) { - final resource = error['resource']; - final field = error['field']; - final code = error['code']; - buff - ..writeln(' Resource: $resource') - ..writeln(' Field $field') - ..write(' Code: $code'); - } - } - throw ValidationFailed(this, buff.toString()); - case 500: - case 502: - case 504: - throw ServerError(this, response.statusCode, message); - } - throw UnknownError(this, message); + ErrorDecoder.decode(this, response); } /// Disposes of this GitHub Instance. @@ -493,16 +438,21 @@ class GitHub { client.close(); } - void _updateRateLimit(Map headers) { + /// Updates rate limit fields from HTTP response [headers]. + void updateRateLimit(Map headers) { if (headers.containsKey(_ratelimitLimitHeader)) { - _rateLimitLimit = int.parse(headers[_ratelimitLimitHeader]!); - _rateLimitRemaining = int.parse(headers[_ratelimitRemainingHeader]!); - _rateLimitReset = int.parse(headers[_ratelimitResetHeader]!); + _rateLimitLimit = int.tryParse(headers[_ratelimitLimitHeader] ?? ''); + _rateLimitRemaining = + int.tryParse(headers[_ratelimitRemainingHeader] ?? ''); + _rateLimitReset = int.tryParse(headers[_ratelimitResetHeader] ?? ''); } } } -void _applyExpandos(dynamic target, http.Response response) { +void _applyExpandos(Object? target, http.Response response) { + if (target == null || target is String || target is num || target is bool) { + return; + } _etagExpando[target] = response.headers['etag']; if (response.headers['date'] != null) { _dateExpando[target] = http_parser.parseHttpDate(response.headers['date']!); diff --git a/lib/src/common/transport/error_decoder.dart b/lib/src/common/transport/error_decoder.dart new file mode 100644 index 00000000..67eae034 --- /dev/null +++ b/lib/src/common/transport/error_decoder.dart @@ -0,0 +1,233 @@ +// ignore_for_file: avoid_classes_with_only_static_members + +import 'dart:convert'; +import 'package:http/http.dart' as http; +import '../github.dart'; +import '../util/errors.dart'; + +/// Helper to parse, redact, and construct typed GitHub exceptions from HTTP error responses. +class ErrorDecoder { + static final RegExp _tokenRegex = RegExp( + r'(?:gh[pousr]_[A-Za-z0-9_]{20,}|github_pat_[A-Za-z0-9_]{20,})', + ); + static final RegExp _authHeaderRegex = RegExp( + r'((?:bearer|token)\s+)[A-Za-z0-9_\-\.]+', + caseSensitive: false, + ); + + static const int maxBodyLength = 2000; + + /// Redacts sensitive authentication tokens from [input]. + static String redact(String input) { + return input + .replaceAll(_tokenRegex, '[REDACTED_TOKEN]') + .replaceAllMapped(_authHeaderRegex, (m) => '${m.group(1)}[REDACTED]'); + } + + /// Truncates [body] safely and redacts credentials. + static String sanitizeBody(String body) { + var truncated = body; + if (truncated.length > maxBodyLength) { + truncated = '${truncated.substring(0, maxBodyLength)}... [TRUNCATED]'; + } + return redact(truncated); + } + + /// Safely decodes an HTTP response into a typed [GitHubError] and throws it. + static Never decode( + GitHub github, + http.Response response, { + String? requestUrl, + }) { + final sanitizedBody = sanitizeBody(response.body); + final contentType = response.headers['content-type'] ?? ''; + final requestId = response.headers['x-github-request-id']; + + String? message; + List>? errors; + + if (contentType.contains('application/json') || + response.body.trim().startsWith('{')) { + try { + final decoded = jsonDecode(response.body); + if (decoded is Map) { + message = decoded['message']?.toString(); + if (decoded['errors'] != null) { + final rawErrors = decoded['errors']; + if (rawErrors is List) { + errors = rawErrors.map((e) { + if (e is Map) { + return Map.from(e); + } else if (e is Map) { + return e.map((k, v) => MapEntry(k.toString(), v)); + } + return {'message': e.toString()}; + }).toList(); + } else if (rawErrors is Map) { + errors = [ + rawErrors.map((k, v) => MapEntry(k.toString(), v)), + ]; + } else { + errors = [ + {'code': rawErrors.toString()}, + ]; + } + } + } + } catch (_) { + // Fall back gracefully on malformed JSON without crashing + } + } + + if (message != null) { + message = redact(message); + } + + final effectiveMessage = message ?? + (sanitizedBody.isNotEmpty + ? sanitizedBody + : 'HTTP ${response.statusCode}'); + + final rateRemaining = response.headers['x-ratelimit-remaining']; + final isRateLimitExceeded = response.statusCode == 429 || + (response.statusCode == 403 && + ((rateRemaining != null && rateRemaining == '0') || + effectiveMessage.toLowerCase().contains('rate limit'))); + + if (isRateLimitExceeded) { + DateTime? resetDate; + final resetHeader = response.headers['x-ratelimit-reset']; + if (resetHeader != null) { + final resetEpoch = int.tryParse(resetHeader.trim()); + if (resetEpoch != null) { + resetDate = DateTime.fromMillisecondsSinceEpoch(resetEpoch * 1000); + } + } + final limit = int.tryParse(response.headers['x-ratelimit-limit'] ?? ''); + final remaining = int.tryParse(rateRemaining ?? ''); + + throw RateLimitHit( + github, + message: effectiveMessage, + reset: resetDate, + limit: limit, + remaining: remaining, + apiUrl: requestUrl, + statusCode: response.statusCode, + responseBody: sanitizedBody, + responseHeaders: response.headers, + requestId: requestId, + ); + } + + switch (response.statusCode) { + case 400: + if (message == 'Problems parsing JSON' || + message == 'Body should be a JSON Hash') { + throw InvalidJSON( + github, + message, + requestUrl, + response.headers, + sanitizedBody, + ); + } + throw BadRequest( + github, + effectiveMessage, + requestUrl, + response.headers, + sanitizedBody, + ); + case 401: + throw NotAuthenticated( + github, + effectiveMessage, + requestUrl, + response.headers, + sanitizedBody, + ); + case 403: + throw AccessForbidden( + github, + effectiveMessage, + requestUrl, + response.headers, + sanitizedBody, + ); + case 404: + throw NotFound( + github, + effectiveMessage.isNotEmpty + ? effectiveMessage + : 'Requested Resource was Not Found', + apiUrl: requestUrl, + responseBody: sanitizedBody, + responseHeaders: response.headers, + requestId: requestId, + ); + case 409: + throw Conflict( + github, + effectiveMessage, + requestUrl, + response.headers, + sanitizedBody, + ); + case 422: + final buff = StringBuffer(); + buff.writeln(); + buff.writeln(' Message: $effectiveMessage'); + if (errors != null) { + buff.writeln(' Errors:'); + for (final error in errors) { + final resource = error['resource']; + final field = error['field']; + final code = error['code']; + final errMessage = error['message']; + buff.writeln( + ' ${resource != null ? 'Resource: $resource ' : ''}${field != null ? 'Field: $field ' : ''}${code != null ? 'Code: $code ' : ''}${errMessage != null ? 'Message: $errMessage' : ''}' + .trimRight()); + } + } + throw ValidationFailed( + github, + buff.toString().trim(), + errors, + requestUrl, + response.headers, + sanitizedBody, + ); + case 500: + case 502: + case 504: + throw ServerError( + github, + response.statusCode, + message, + apiUrl: requestUrl, + responseHeaders: response.headers, + responseBody: sanitizedBody, + requestId: requestId, + ); + case 503: + throw ServiceUnavailable( + github, + message, + requestUrl, + response.headers, + sanitizedBody, + requestId, + ); + default: + throw UnknownError( + github, + effectiveMessage, + requestUrl, + response.headers, + sanitizedBody, + response.statusCode, + ); + } + } +} diff --git a/lib/src/common/transport/request.dart b/lib/src/common/transport/request.dart new file mode 100644 index 00000000..8d749f83 --- /dev/null +++ b/lib/src/common/transport/request.dart @@ -0,0 +1,46 @@ +import '../generated/rest_contracts.g.dart'; + +/// A structured, type-safe API request description. +class ApiRequest { + /// HTTP method (e.g. 'GET', 'POST', 'PATCH', 'PUT', 'DELETE'). + final String method; + + /// Request path or absolute URL. + final String path; + + /// HTTP headers to send with the request. + final Map headers; + + /// Query parameters to encode into the URL. + final Map params; + + /// Request body (JSON-encodable Object, `List` bytes, or `null`). + final Object? body; + + /// Set of accepted HTTP status codes considered successful. + /// If `null`, any status code is returned without error decoding. + final Set? successStatuses; + + /// Associated OpenAPI operation contract, if known. + final RestOperationContract? contract; + + /// Optional per-request timeout. + final Duration? timeout; + + const ApiRequest({ + required this.method, + required this.path, + this.headers = const {}, + this.params = const {}, + this.body, + this.successStatuses = const {200}, + this.contract, + this.timeout, + }); + + /// True if the HTTP method is idempotent according to RFC 9110. + bool get isIdempotent { + final m = method.toUpperCase(); + return m == 'GET' || m == 'HEAD' || m == 'PUT' || m == 'DELETE'; + } +} diff --git a/lib/src/common/transport/retry_policy.dart b/lib/src/common/transport/retry_policy.dart new file mode 100644 index 00000000..9d28516d --- /dev/null +++ b/lib/src/common/transport/retry_policy.dart @@ -0,0 +1,148 @@ +import 'dart:async'; +import 'dart:io'; +import 'dart:math' as math; +import 'package:http/http.dart' as http; +import 'request.dart'; + +typedef Sleeper = Future Function(Duration duration); +typedef Clock = DateTime Function(); +typedef RandomGenerator = double Function(); + +/// Policy governing request retries, exponential backoff, and rate-limit delays. +class RetryPolicy { + /// Maximum number of retry attempts for idempotent operations. + final int maxRetries; + + /// Initial backoff delay for the first retry. + final Duration initialDelay; + + /// Maximum backoff delay allowed between retries. + final Duration maxDelay; + + /// Multiplier for exponential backoff calculation. + final double backoffMultiplier; + + /// Whether to retry requests that hit HTTP 429 or secondary rate limits. + final bool retryOnRateLimit; + + /// Injectable sleeper function (defaults to `Future.delayed`). + final Sleeper sleeper; + + /// Injectable clock function (defaults to `DateTime.now`). + final Clock clock; + + /// Injectable random generator for jitter (returns value in [0.0, 1.0)). + final RandomGenerator? random; + + const RetryPolicy({ + this.maxRetries = 3, + this.initialDelay = const Duration(milliseconds: 250), + this.maxDelay = const Duration(seconds: 30), + this.backoffMultiplier = 2.0, + this.retryOnRateLimit = true, + this.sleeper = Future.delayed, + this.clock = DateTime.now, + this.random, + }); + + /// No-retry policy for tests or callers desiring fail-fast behavior. + static const RetryPolicy noRetries = RetryPolicy(maxRetries: 0); + + /// Determines whether [error] or [response] warrants a retry on [attempt] (0-indexed). + bool shouldRetry({ + required ApiRequest request, + required int attempt, + http.Response? response, + Object? error, + }) { + if (attempt >= maxRetries) { + return false; + } + + // By default, retry only idempotent requests + if (!request.isIdempotent) { + return false; + } + + if (error != null) { + if (error is TimeoutException || + error is SocketException || + error is http.ClientException) { + return true; + } + return false; + } + + if (response != null) { + final code = response.statusCode; + if (code == 502 || code == 503 || code == 504) { + return true; + } + if (retryOnRateLimit && + (code == 429 || _isSecondaryRateLimit(response))) { + return true; + } + } + + return false; + } + + /// Calculates delay before attempt [attempt] (0-indexed), honoring `Retry-After` if present. + Duration delayFor({ + required int attempt, + http.Response? response, + }) { + if (response != null) { + final retryAfterHeader = response.headers['retry-after']; + if (retryAfterHeader != null) { + final seconds = int.tryParse(retryAfterHeader.trim()); + if (seconds != null) { + final d = Duration(seconds: seconds); + return d > maxDelay ? maxDelay : d; + } + try { + final date = HttpDate.parse(retryAfterHeader); + final diff = date.difference(clock()); + if (diff > Duration.zero) { + return diff > maxDelay ? maxDelay : diff; + } + } catch (_) {} + } + + // Check x-ratelimit-reset if 429 or secondary rate limit + if (response.statusCode == 429 || _isSecondaryRateLimit(response)) { + final resetHeader = response.headers['x-ratelimit-reset']; + if (resetHeader != null) { + final resetEpoch = int.tryParse(resetHeader.trim()); + if (resetEpoch != null) { + final resetDate = + DateTime.fromMillisecondsSinceEpoch(resetEpoch * 1000); + final diff = resetDate.difference(clock()); + if (diff > Duration.zero && diff <= maxDelay) { + return diff; + } + } + } + } + } + + // Exponential backoff with ±20% jitter + final exponentialMs = + initialDelay.inMilliseconds * math.pow(backoffMultiplier, attempt); + final jitterFactor = + 0.8 + ((random?.call() ?? math.Random().nextDouble()) * 0.4); + final delayMs = (exponentialMs * jitterFactor).round(); + + final capped = math.min(delayMs, maxDelay.inMilliseconds); + return Duration(milliseconds: capped); + } + + bool _isSecondaryRateLimit(http.Response response) { + if (response.statusCode != 403) { + return false; + } + final body = response.body.toLowerCase(); + return body.contains('secondary rate limit') || + body.contains('rate limit exceeded'); + } +} diff --git a/lib/src/common/transport/transport.dart b/lib/src/common/transport/transport.dart new file mode 100644 index 00000000..693e2ed3 --- /dev/null +++ b/lib/src/common/transport/transport.dart @@ -0,0 +1,280 @@ +import 'dart:async'; +import 'dart:convert'; +import 'package:http/http.dart' as http; +import '../github.dart'; +import '../util/auth.dart'; +import '../util/errors.dart'; +import '../util/json.dart'; +import 'error_decoder.dart'; +import 'request.dart'; +import 'retry_policy.dart'; + +typedef ResponseCallback = void Function(http.Response response); + +/// High-level transport orchestrating URI construction, credential boundary checks, +/// serialization, retry loops, rate-limit tracking, and typed error decoding. +class HttpTransport { + final GitHub github; + final http.Client client; + final RetryPolicy retryPolicy; + final Set trustedOrigins; + final bool allowInsecureAuth; + final String endpoint; + + HttpTransport({ + required this.github, + required this.client, + required this.trustedOrigins, + this.retryPolicy = const RetryPolicy(), + this.allowInsecureAuth = false, + this.endpoint = 'https://api.github.com', + }); + + /// Resolves an [ApiRequest] into an absolute, normalized [Uri]. + Uri resolveUri( + String rawPath, { + Map params = const {}, + Map pathParams = const {}, + Set multiSegmentParams = const {}, + }) { + var expandedPath = rawPath; + + // Substitute path parameters e.g. {owner}, {repo}, {+path} + for (final entry in pathParams.entries) { + final key = entry.key; + final value = entry.value; + final isMultiSegment = + multiSegmentParams.contains(key) || key.startsWith('+'); + final encoded = + isMultiSegment ? Uri.encodeFull(value) : Uri.encodeComponent(value); + expandedPath = expandedPath + .replaceAll('{$key}', encoded) + .replaceAll('{+$key}', encoded); + } + + Uri targetUri; + if (expandedPath.startsWith('http://') || + expandedPath.startsWith('https://')) { + targetUri = Uri.parse(expandedPath); + } else { + final normalizedEndpoint = endpoint.endsWith('/') + ? endpoint.substring(0, endpoint.length - 1) + : endpoint; + final normalizedPath = + expandedPath.startsWith('/') ? expandedPath : '/$expandedPath'; + + targetUri = Uri.parse('$normalizedEndpoint$normalizedPath'); + } + + // Merge query parameters + if (params.isNotEmpty) { + final currentQuery = + Map>.from(targetUri.queryParametersAll); + for (final entry in params.entries) { + if (entry.value == null) { + continue; + } + final key = entry.key; + final value = entry.value; + if (value is Iterable) { + final list = currentQuery.putIfAbsent(key, () => []); + for (final item in value) { + if (item != null) { + list.add(item is Enum ? item.name : item.toString()); + } + } + } else { + final list = currentQuery.putIfAbsent(key, () => []); + list.add(value is Enum ? value.name : value.toString()); + } + } + + targetUri = targetUri.replace( + queryParameters: currentQuery.isEmpty ? null : currentQuery); + } + + return targetUri; + } + + /// Validates security constraints against [targetUri]. + void validateSecurityBoundaries(Uri targetUri, Authentication auth) { + if (targetUri.scheme != 'https' && + !allowInsecureAuth && + !auth.isAnonymous) { + throw GitHubError( + github, + 'Insecure HTTP authentication is disallowed by default. ' + 'Set allowInsecureAuth: true only for local testing.', + ); + } + } + + /// Prepares the headers map for [request], injecting User-Agent and Authorization. + Map prepareHeaders( + ApiRequest request, + Uri targetUri, + Authentication auth, + ) { + final headers = Map.from(request.headers); + + // Apply GitHub required User-Agent + headers.putIfAbsent('User-Agent', () => auth.username ?? 'github.dart'); + + // Apply Authorization if origin is trusted + if (trustedOrigins.contains(targetUri.origin)) { + final authHeader = auth.authorizationHeaderValue(); + if (authHeader != null) { + headers.putIfAbsent('Authorization', () => authHeader); + } + } + + // Content length for empty PUT/POST + if ((request.method == 'PUT' || request.method == 'POST') && + request.body == null) { + headers.putIfAbsent('Content-Length', () => '0'); + } + + return headers; + } + + /// Executes [request], performing retries and typed error handling. + Future execute( + ApiRequest request, { + ResponseCallback? fail, + }) async { + // If rate limit remaining is 0, wait for reset window if within reasonable max delay + if (github.rateLimitRemaining != null && + github.rateLimitRemaining! <= 0 && + github.rateLimitReset != null) { + final now = retryPolicy.clock(); + if (github.rateLimitReset!.isAfter(now)) { + final waitTime = github.rateLimitReset!.difference(now); + if (waitTime <= retryPolicy.maxDelay) { + await retryPolicy.sleeper(waitTime); + } + } + } + + final auth = github.auth; + final targetUri = resolveUri( + request.path, + params: request.params, + ); + + validateSecurityBoundaries(targetUri, auth); + final headers = prepareHeaders(request, targetUri, auth); + + http.Response? lastResponse; + Object? lastError; + + for (var attempt = 0; attempt <= retryPolicy.maxRetries; attempt++) { + try { + final httpRequest = http.Request(request.method, targetUri); + httpRequest.headers.addAll(headers); + + if (request.body != null) { + final body = request.body; + if (body is List) { + httpRequest.bodyBytes = body; + } else if (body is String) { + httpRequest.body = body; + } else { + httpRequest.headers.putIfAbsent( + 'Content-Type', () => 'application/json; charset=utf-8'); + httpRequest.body = jsonEncode(body); + } + } + + var sendFuture = client.send(httpRequest); + if (request.timeout != null) { + sendFuture = sendFuture.timeout(request.timeout!); + } + + final streamed = await sendFuture; + final response = await http.Response.fromStream(streamed); + lastResponse = response; + + // Update rate limit metadata on parent github instance + github.updateRateLimit(response.headers); + + // Check if status is accepted + if (request.successStatuses == null || + request.successStatuses!.contains(response.statusCode)) { + return response; + } + + // Check if retry policy triggers on this response + if (retryPolicy.shouldRetry( + request: request, + attempt: attempt, + response: response, + )) { + final delay = + retryPolicy.delayFor(attempt: attempt, response: response); + await retryPolicy.sleeper(delay); + continue; + } + + // Failed status without retry + if (fail != null) { + fail(response); + } + ErrorDecoder.decode(github, response, requestUrl: targetUri.toString()); + } catch (e) { + lastError = e; + if (e is GitHubError) { + rethrow; + } + + if (retryPolicy.shouldRetry( + request: request, + attempt: attempt, + error: e, + )) { + final delay = retryPolicy.delayFor(attempt: attempt); + await retryPolicy.sleeper(delay); + continue; + } + + throw GitHubError( + github, + 'Network request failed: $e', + apiUrl: targetUri.toString(), + source: e, + ); + } + } + + if (lastResponse != null) { + if (fail != null) { + fail(lastResponse); + } + ErrorDecoder.decode(github, lastResponse, + requestUrl: targetUri.toString()); + } + + throw GitHubError( + github, + 'Request failed after ${retryPolicy.maxRetries} retries: $lastError', + apiUrl: targetUri.toString(), + source: lastError, + ); + } + + /// Convenience method to execute a request and decode the JSON body. + Future executeJson( + ApiRequest request, { + JSONConverter? convert, + ResponseCallback? fail, + }) async { + final response = await execute(request, fail: fail); + if (response.statusCode == 204 || response.body.isEmpty) { + return null as T; + } + final decoded = jsonDecode(response.body); + if (convert != null) { + return convert(decoded) as T; + } + return decoded as T; + } +} diff --git a/lib/src/common/url_shortener_service.dart b/lib/src/common/url_shortener_service.dart index 60db2958..9fb20b11 100644 --- a/lib/src/common/url_shortener_service.dart +++ b/lib/src/common/url_shortener_service.dart @@ -1,15 +1,20 @@ import 'dart:async'; import 'package:github/src/common.dart'; -/// The [UrlShortenerService] provides a handy method to access GitHub's -/// url shortener. +/// The [UrlShortenerService] provides a method to access GitHub's +/// legacy git.io url shortener. /// -/// API docs: https://github.com/blog/985-git-io-github-url-shortener +/// NOTE: The git.io service was deprecated and discontinued by GitHub in 2022. +/// This service is retained for backwards compatibility only. +@Deprecated( + 'git.io was discontinued by GitHub in 2022 and is no longer operational.') class UrlShortenerService extends Service { UrlShortenerService(super.github); /// Shortens the provided [url]. An optional [code] can be provided to create /// your own vanity URL. + @Deprecated( + 'git.io was discontinued by GitHub in 2022 and is no longer operational.') Future shortenUrl(String url, {String? code}) { final params = {}; @@ -19,14 +24,19 @@ class UrlShortenerService extends Service { params['code'] = code; } - return github - .request('POST', 'http://git.io/', params: params) - .then((response) { + // Never send authentication credentials to git.io; use HTTPS. + return github.request('POST', 'https://git.io/', + params: params, headers: {}).then((response) { if (response.statusCode != StatusCodes.CREATED) { throw GitHubError(github, 'Failed to create shortened url!'); } - return response.headers['Location']!.split('/').last; + final location = + response.headers['location'] ?? response.headers['Location']; + if (location == null) { + throw GitHubError(github, 'Missing Location header in response'); + } + return location.split('/').last; }); } } diff --git a/lib/src/common/util/errors.dart b/lib/src/common/util/errors.dart index 14625a6c..2c6529cf 100644 --- a/lib/src/common/util/errors.dart +++ b/lib/src/common/util/errors.dart @@ -6,8 +6,21 @@ class GitHubError implements Exception { final String? apiUrl; final GitHub github; final Object? source; + final int? statusCode; + final String? responseBody; + final Map? responseHeaders; + final String? requestId; - const GitHubError(this.github, this.message, {this.apiUrl, this.source}); + const GitHubError( + this.github, + this.message, { + this.apiUrl, + this.source, + this.statusCode, + this.responseBody, + this.responseHeaders, + this.requestId, + }); @override String toString() => 'GitHub Error: $message'; @@ -26,12 +39,29 @@ class NotReady extends GitHubError { class NotFound extends GitHubError { const NotFound( super.github, - String super.msg, - ); + String super.msg, { + super.apiUrl, + super.source, + super.statusCode = 404, + super.responseBody, + super.responseHeaders, + super.requestId, + }); } class BadRequest extends GitHubError { - const BadRequest(super.github, [super.msg = 'Not Found']); + const BadRequest( + super.github, [ + super.message = 'Bad Request', + String? apiUrl, + Map? responseHeaders, + String? responseBody, + ]) : super( + apiUrl: apiUrl, + statusCode: 400, + responseBody: responseBody, + responseHeaders: responseHeaders, + ); } /// GitHub Repository was not found @@ -64,39 +94,166 @@ class TeamNotFound extends NotFound { : super(github, 'Team Not Found: $id'); } -/// Access was forbidden to a resource +/// Access was forbidden to a resource (HTTP 403) class AccessForbidden extends GitHubError { - const AccessForbidden(GitHub github) : super(github, 'Access Forbidden'); + const AccessForbidden( + super.github, [ + super.message = 'Access Forbidden', + String? apiUrl, + Map? responseHeaders, + String? responseBody, + ]) : super( + apiUrl: apiUrl, + statusCode: 403, + responseBody: responseBody, + responseHeaders: responseHeaders, + ); } -/// Client hit the rate limit. +/// Client hit the rate limit (HTTP 429 or HTTP 403 rate limit exceeded). class RateLimitHit extends GitHubError { - const RateLimitHit(GitHub github) : super(github, 'Rate Limit Hit'); + final DateTime? reset; + final int? limit; + final int? remaining; + + const RateLimitHit( + GitHub github, { + String? message, + this.reset, + this.limit, + this.remaining, + String? apiUrl, + int statusCode = 429, + String? responseBody, + Map? responseHeaders, + String? requestId, + }) : super( + github, + message ?? 'Rate Limit Hit', + apiUrl: apiUrl, + statusCode: statusCode, + responseBody: responseBody, + responseHeaders: responseHeaders, + requestId: requestId, + ); } -/// A GitHub Server Error +/// Resource conflict (HTTP 409) +class Conflict extends GitHubError { + const Conflict( + super.github, [ + super.message = 'Conflict', + String? apiUrl, + Map? responseHeaders, + String? responseBody, + ]) : super( + apiUrl: apiUrl, + statusCode: 409, + responseBody: responseBody, + responseHeaders: responseHeaders, + ); +} + +/// A GitHub Server Error (HTTP 500, 502, 504) class ServerError extends GitHubError { - ServerError(GitHub github, int statusCode, String? message) - : super(github, '${message ?? 'Server Error'} ($statusCode)'); + ServerError( + GitHub github, + int statusCode, + String? message, { + String? apiUrl, + Map? responseHeaders, + String? responseBody, + String? requestId, + }) : super( + github, + '${message ?? 'Server Error'} ($statusCode)', + statusCode: statusCode, + apiUrl: apiUrl, + responseHeaders: responseHeaders, + responseBody: responseBody, + requestId: requestId, + ); +} + +/// Service Unavailable (HTTP 503) +class ServiceUnavailable extends ServerError { + ServiceUnavailable( + GitHub github, [ + String? message, + String? apiUrl, + Map? responseHeaders, + String? responseBody, + String? requestId, + ]) : super( + github, + 503, + message ?? 'Service Unavailable', + apiUrl: apiUrl, + responseHeaders: responseHeaders, + responseBody: responseBody, + requestId: requestId, + ); } /// An Unknown Error class UnknownError extends GitHubError { - const UnknownError(GitHub github, [String? message]) - : super(github, message ?? 'Unknown Error'); + const UnknownError( + GitHub github, [ + String? message, + String? apiUrl, + Map? responseHeaders, + String? responseBody, + int? statusCode, + ]) : super( + github, + message ?? 'Unknown Error', + apiUrl: apiUrl, + statusCode: statusCode, + responseHeaders: responseHeaders, + responseBody: responseBody, + ); } -/// GitHub Client was not authenticated +/// GitHub Client was not authenticated (HTTP 401) class NotAuthenticated extends GitHubError { - const NotAuthenticated(GitHub github) - : super(github, 'Client not Authenticated'); + const NotAuthenticated( + super.github, [ + super.message = 'Client not Authenticated', + String? apiUrl, + Map? responseHeaders, + String? responseBody, + ]) : super( + apiUrl: apiUrl, + statusCode: 401, + responseBody: responseBody, + responseHeaders: responseHeaders, + ); } class InvalidJSON extends BadRequest { - const InvalidJSON(super.github, [super.message = 'Invalid JSON']); + const InvalidJSON( + super.github, [ + super.message = 'Invalid JSON', + super.apiUrl, + super.responseHeaders, + super.responseBody, + ]); } class ValidationFailed extends GitHubError { - const ValidationFailed(super.github, - [String super.message = 'Validation Failed']); + final List>? errors; + + const ValidationFailed( + super.github, [ + super.message = 'Validation Failed', + this.errors, + String? apiUrl, + Map? responseHeaders, + String? responseBody, + ]) : super( + apiUrl: apiUrl, + statusCode: 422, + responseHeaders: responseHeaders, + responseBody: responseBody, + ); } diff --git a/lib/src/server/hooks.dart b/lib/src/server/hooks.dart index ae3fa0bf..b7941e2a 100644 --- a/lib/src/server/hooks.dart +++ b/lib/src/server/hooks.dart @@ -1,7 +1,9 @@ import 'dart:async'; import 'dart:convert'; import 'dart:io'; +import 'dart:typed_data'; +import 'package:crypto/crypto.dart'; import 'package:json_annotation/json_annotation.dart'; import '../common.dart'; @@ -9,63 +11,231 @@ import '../common/model/changes.dart'; part 'hooks.g.dart'; +/// Middleware for processing GitHub webhooks securely. class HookMiddleware { - // TODO: Close this, but where? + /// Webhook secret for HMAC-SHA256 signature verification. + /// If provided, all incoming requests must have a valid `X-Hub-Signature-256` header. + final String? secret; + + /// Maximum allowed payload size in bytes. Defaults to 10 MB. + final int maxBodySize; + + /// Optional hook for replay attack detection based on delivery ID. + /// Returns `true` if the delivery ID is fresh/acceptable, or `false` if it is a replay. + final bool Function(String deliveryId)? onReplayCheck; + final StreamController _eventController = - StreamController(); + StreamController.broadcast(); + Stream get onEvent => _eventController.stream; - void handleHookRequest(HttpRequest request) { + HookMiddleware({ + this.secret, + this.maxBodySize = 10 * 1024 * 1024, + this.onReplayCheck, + }); + + /// Closes the webhook event stream and releases resources. + Future close() => _eventController.close(); + + /// Constant-time string equality check to prevent timing attacks. + static bool constantTimeEquals(String a, String b) { + if (a.length != b.length) { + return false; + } + var result = 0; + for (var i = 0; i < a.length; i++) { + result |= a.codeUnitAt(i) ^ b.codeUnitAt(i); + } + return result == 0; + } + + /// Verifies an incoming webhook HMAC-SHA256 signature. + static bool verifySignature( + String secret, List payloadBytes, String? signatureHeader) { + if (signatureHeader == null || !signatureHeader.startsWith('sha256=')) { + return false; + } + final expectedHex = signatureHeader.substring(7); + final hmac = Hmac(sha256, utf8.encode(secret)); + final calculated = hmac.convert(payloadBytes).toString(); + return constantTimeEquals( + calculated.toLowerCase(), expectedHex.toLowerCase()); + } + + /// Handles an incoming webhook HTTP request. + Future handleHookRequest(HttpRequest request) async { if (request.method != 'POST') { request.response - ..write('Only POST is Supported') + ..statusCode = HttpStatus.methodNotAllowed + ..headers.contentType = ContentType.json + ..write(jsonEncode({ + 'error': 'Method Not Allowed', + 'message': 'Only POST is Supported' + })) ..close(); return; } - if (request.headers.value('X-GitHub-Event') == null) { + final eventHeader = request.headers.value('X-GitHub-Event'); + if (eventHeader == null || eventHeader.trim().isEmpty) { request.response - ..write('X-GitHub-Event must be specified.') + ..statusCode = HttpStatus.badRequest + ..headers.contentType = ContentType.json + ..write(jsonEncode({ + 'error': 'Bad Request', + 'message': 'X-GitHub-Event must be specified.' + })) ..close(); return; } - const Utf8Decoder().bind(request).join().then((content) { - _eventController.add(HookEvent.fromJson( - request.headers.value('X-GitHub-Event'), - jsonDecode(content) as Map?)); + final deliveryId = request.headers.value('X-GitHub-Delivery'); + if (deliveryId != null && onReplayCheck != null) { + final isFresh = onReplayCheck!(deliveryId); + if (!isFresh) { + request.response + ..statusCode = HttpStatus.conflict + ..headers.contentType = ContentType.json + ..write(jsonEncode( + {'error': 'Conflict', 'message': 'Duplicate delivery'})) + ..close(); + return; + } + } + + final builder = BytesBuilder(copy: false); + var bytesReceived = 0; + + try { + await for (final chunk in request) { + bytesReceived += chunk.length; + if (bytesReceived > maxBodySize) { + request.response + ..statusCode = HttpStatus.requestEntityTooLarge + ..headers.contentType = ContentType.json + ..write(jsonEncode({ + 'error': 'Payload Too Large', + 'message': + 'Payload exceeds maximum allowed size of $maxBodySize bytes' + })) + ..close(); + return; + } + builder.add(chunk); + } + } catch (_) { request.response - ..write(GitHubJson.encode({'handled': _eventController.hasListener})) + ..statusCode = HttpStatus.badRequest + ..headers.contentType = ContentType.json + ..write(jsonEncode( + {'error': 'Bad Request', 'message': 'Failed to read request body'})) ..close(); - }); + return; + } + + final rawBytes = builder.takeBytes(); + + if (secret != null) { + final signatureHeader = request.headers.value('X-Hub-Signature-256'); + if (!verifySignature(secret!, rawBytes, signatureHeader)) { + request.response + ..statusCode = HttpStatus.unauthorized + ..headers.contentType = ContentType.json + ..write(jsonEncode( + {'error': 'Unauthorized', 'message': 'Invalid signature'})) + ..close(); + return; + } + } + + Map? bodyJson; + try { + final text = utf8.decode(rawBytes); + if (text.isNotEmpty) { + final decoded = jsonDecode(text); + if (decoded is Map) { + bodyJson = decoded; + } else if (decoded is Map) { + bodyJson = decoded.cast(); + } + } + } catch (_) { + request.response + ..statusCode = HttpStatus.badRequest + ..headers.contentType = ContentType.json + ..write(jsonEncode( + {'error': 'Bad Request', 'message': 'Invalid JSON body'})) + ..close(); + return; + } + + _eventController.add(HookEvent.fromJson(eventHeader, bodyJson)); + request.response + ..statusCode = HttpStatus.ok + ..headers.contentType = ContentType.json + ..write(GitHubJson.encode({'handled': _eventController.hasListener})) + ..close(); } } +/// A standalone HTTP server for receiving GitHub webhooks. class HookServer extends HookMiddleware { final String host; final int port; - late HttpServer _server; - - HookServer(this.port, [this.host = '0.0.0.0']); + HttpServer? _server; + + HookServer( + this.port, [ + this.host = '127.0.0.1', + String? secret, + int maxBodySize = 10 * 1024 * 1024, + bool Function(String deliveryId)? onReplayCheck, + ]) : super( + secret: secret, + maxBodySize: maxBodySize, + onReplayCheck: onReplayCheck, + ); + + HookServer.options({ + required this.port, + this.host = '127.0.0.1', + super.secret, + super.maxBodySize, + super.onReplayCheck, + }); - void start() { - HttpServer.bind(host, port).then((HttpServer server) { - _server = server; - server.listen((request) { + /// Starts the HTTP server and returns the bound [HttpServer] instance. + Future start() async { + final server = await HttpServer.bind(host, port); + _server = server; + server.listen((request) async { + try { if (request.uri.path == '/hook') { - handleHookRequest(request); + await handleHookRequest(request); } else { request.response - ..statusCode = 404 + ..statusCode = HttpStatus.notFound ..write('404 - Not Found') ..close(); } - }); + } catch (_) { + try { + request.response + ..statusCode = HttpStatus.internalServerError + ..close(); + } catch (_) {} + } }); + return server; } - Future stop() => _server.close(); + /// Stops the HTTP server and closes the webhook event stream. + Future stop() async { + await _server?.close(force: true); + await close(); + } } class HookEvent { diff --git a/lib/src/server/xplat_server.dart b/lib/src/server/xplat_server.dart index a6c85d02..3ddc8968 100644 --- a/lib/src/server/xplat_server.dart +++ b/lib/src/server/xplat_server.dart @@ -9,7 +9,8 @@ export 'hooks.dart'; /// /// Checks all the environment variables in [COMMON_GITHUB_TOKEN_ENV_KEYS] for tokens. /// If the above fails, the GITHUB_USERNAME and GITHUB_PASSWORD keys will be checked. -Authentication findAuthenticationFromEnvironment() { +Authentication findAuthenticationFromEnvironment( + {bool allowQueryAuth = false}) { if (Platform.isMacOS) { final result = Process.runSync( 'security', const ['find-internet-password', '-g', '-s', 'github.com']); diff --git a/test/unit/security_test.dart b/test/unit/security_test.dart new file mode 100644 index 00000000..84f6677c --- /dev/null +++ b/test/unit/security_test.dart @@ -0,0 +1,305 @@ +import 'dart:async'; +import 'dart:convert'; +import 'dart:io'; + +import 'package:crypto/crypto.dart'; +import 'package:github/github.dart'; +import 'package:github/hooks.dart'; +import 'package:http/http.dart' as http; +import 'package:http/testing.dart'; +import 'package:test/test.dart'; + +void main() { + group('Security - Credential boundaries', () { + test('does not send Authorization header to untrusted absolute URL', + () async { + http.Request? capturedRequest; + final mockClient = MockClient((r) async { + capturedRequest = r; + return http.Response('{}', 200, + headers: {'content-type': 'application/json'}); + }); + + final gh = GitHub( + auth: const Authentication.withToken('super-secret-token'), + client: mockClient, + ); + + await gh.request('GET', 'https://attacker.example.com/exfiltrate'); + expect(capturedRequest, isNotNull); + expect(capturedRequest!.headers.containsKey('Authorization'), isFalse); + }); + + test('sends Authorization header to trusted endpoint origin', () async { + http.Request? capturedRequest; + final mockClient = MockClient((r) async { + capturedRequest = r; + return http.Response('{}', 200, + headers: {'content-type': 'application/json'}); + }); + + final gh = GitHub( + auth: const Authentication.withToken('super-secret-token'), + client: mockClient, + ); + + await gh.request('GET', '/user'); + expect(capturedRequest, isNotNull); + expect(capturedRequest!.headers['Authorization'], + 'token super-secret-token'); + }); + + test('sends Authorization header to trusted uploads origin', () async { + http.Request? capturedRequest; + final mockClient = MockClient((r) async { + capturedRequest = r; + return http.Response('{}', 200, + headers: {'content-type': 'application/json'}); + }); + + final gh = GitHub( + auth: const Authentication.withToken('super-secret-token'), + client: mockClient, + ); + + await gh.request('POST', + 'https://uploads.github.com/repos/org/repo/releases/1/assets'); + expect(capturedRequest, isNotNull); + expect(capturedRequest!.headers['Authorization'], + 'token super-secret-token'); + }); + + test('disallows authenticated requests over insecure HTTP by default', + () async { + final mockClient = MockClient((r) async => http.Response('{}', 200)); + + final gh = GitHub( + endpoint: 'http://api.github.com', + auth: const Authentication.withToken('super-secret-token'), + client: mockClient, + ); + + expect( + () => gh.request('GET', '/user'), + throwsA(isA().having( + (e) => e.message, + 'message', + contains('Insecure HTTP authentication is disallowed by default'), + )), + ); + }); + + test( + 'allows authenticated requests over HTTP when allowInsecureAuth is true', + () async { + http.Request? capturedRequest; + final mockClient = MockClient((r) async { + capturedRequest = r; + return http.Response('{}', 200, + headers: {'content-type': 'application/json'}); + }); + + final gh = GitHub( + endpoint: 'http://localhost:3000', + auth: const Authentication.withToken('local-token'), + client: mockClient, + allowInsecureAuth: true, + ); + + await gh.request('GET', '/user'); + expect(capturedRequest, isNotNull); + expect(capturedRequest!.headers['Authorization'], 'token local-token'); + }); + + test('UrlShortenerService does not leak GitHub credentials and uses HTTPS', + () async { + http.Request? capturedRequest; + final mockClient = MockClient((r) async { + capturedRequest = r; + return http.Response('{}', 201, headers: { + 'location': 'https://git.io/abcd', + 'content-type': 'application/json' + }); + }); + + final gh = GitHub( + auth: const Authentication.withToken('super-secret-token'), + client: mockClient, + ); + + // ignore: deprecated_member_use_from_same_package + final shortener = UrlShortenerService(gh); + final code = + // ignore: deprecated_member_use_from_same_package + await shortener.shortenUrl('https://github.com/SpinlockLabs'); + + expect(code, 'abcd'); + expect(capturedRequest, isNotNull); + expect(capturedRequest!.url.scheme, 'https'); + expect(capturedRequest!.url.host, 'git.io'); + expect(capturedRequest!.headers.containsKey('Authorization'), isFalse); + }); + }); + + group('Security - Webhook verification and hardening', () { + late HookServer server; + late int port; + + setUp(() async { + final socket = await ServerSocket.bind('127.0.0.1', 0); + port = socket.port; + await socket.close(); + }); + + tearDown(() async { + await server.stop(); + }); + + test('rejects non-POST methods with 405', () async { + server = HookServer(port, '127.0.0.1'); + await server.start(); + + final client = HttpClient(); + final req = await client.getUrl(Uri.parse('http://127.0.0.1:$port/hook')); + final res = await req.close(); + expect(res.statusCode, HttpStatus.methodNotAllowed); + final body = await utf8.decoder.bind(res).join(); + expect(jsonDecode(body)['error'], 'Method Not Allowed'); + client.close(); + }); + + test('rejects requests missing X-GitHub-Event header with 400', () async { + server = HookServer(port, '127.0.0.1'); + await server.start(); + + final client = HttpClient(); + final req = + await client.postUrl(Uri.parse('http://127.0.0.1:$port/hook')); + req.headers.contentType = ContentType.json; + req.write(jsonEncode({'action': 'created'})); + final res = await req.close(); + expect(res.statusCode, HttpStatus.badRequest); + final body = await utf8.decoder.bind(res).join(); + expect(jsonDecode(body)['error'], 'Bad Request'); + client.close(); + }); + + test( + 'rejects requests when secret configured and signature missing or invalid', + () async { + server = HookServer(port, '127.0.0.1', 'webhook-secret-key'); + await server.start(); + + final client = HttpClient(); + + // Missing signature + var req = await client.postUrl(Uri.parse('http://127.0.0.1:$port/hook')); + req.headers.set('X-GitHub-Event', 'issues'); + req.headers.contentType = ContentType.json; + req.write(jsonEncode({'action': 'opened'})); + var res = await req.close(); + expect(res.statusCode, HttpStatus.unauthorized); + + // Invalid signature + req = await client.postUrl(Uri.parse('http://127.0.0.1:$port/hook')); + req.headers.set('X-GitHub-Event', 'issues'); + req.headers.set('X-Hub-Signature-256', 'sha256=invalidhex0000'); + req.headers.contentType = ContentType.json; + req.write(jsonEncode({'action': 'opened'})); + res = await req.close(); + expect(res.statusCode, HttpStatus.unauthorized); + + client.close(); + }); + + test('accepts requests with valid HMAC-SHA256 signature', () async { + const secret = 'my-webhook-secret'; + server = HookServer(port, '127.0.0.1', secret); + await server.start(); + + HookEvent? receivedEvent; + server.onEvent.listen((e) => receivedEvent = e); + + final payload = jsonEncode({'action': 'opened'}); + final payloadBytes = utf8.encode(payload); + final hmac = Hmac(sha256, utf8.encode(secret)); + final signature = 'sha256=${hmac.convert(payloadBytes)}'; + + final client = HttpClient(); + final req = + await client.postUrl(Uri.parse('http://127.0.0.1:$port/hook')); + req.headers.set('X-GitHub-Event', 'issues'); + req.headers.set('X-Hub-Signature-256', signature); + req.headers.contentType = ContentType.json; + req.add(payloadBytes); + final res = await req.close(); + + expect(res.statusCode, HttpStatus.ok); + await Future.delayed(const Duration(milliseconds: 20)); + expect(receivedEvent, isA()); + expect((receivedEvent as IssueEvent).action, 'opened'); + client.close(); + }); + + test('rejects requests exceeding maxBodySize with 413', () async { + server = HookServer(port, '127.0.0.1', null, 50); // max 50 bytes + await server.start(); + + final client = HttpClient(); + final req = + await client.postUrl(Uri.parse('http://127.0.0.1:$port/hook')); + req.headers.set('X-GitHub-Event', 'issues'); + req.headers.contentType = ContentType.json; + req.write(jsonEncode({'action': 'a' * 100})); + final res = await req.close(); + + expect(res.statusCode, HttpStatus.requestEntityTooLarge); + client.close(); + }); + + test('rejects malformed JSON with 400', () async { + server = HookServer(port, '127.0.0.1'); + await server.start(); + + final client = HttpClient(); + final req = + await client.postUrl(Uri.parse('http://127.0.0.1:$port/hook')); + req.headers.set('X-GitHub-Event', 'issues'); + req.headers.contentType = ContentType.json; + req.write('{malformed json string...'); + final res = await req.close(); + + expect(res.statusCode, HttpStatus.badRequest); + client.close(); + }); + + test('detects replay attack when onReplayCheck returns false', () async { + final seenDeliveries = {}; + server = + HookServer(port, '127.0.0.1', null, 1024 * 1024, seenDeliveries.add); + await server.start(); + + final client = HttpClient(); + + // First delivery: succeeds + var req = await client.postUrl(Uri.parse('http://127.0.0.1:$port/hook')); + req.headers.set('X-GitHub-Event', 'issues'); + req.headers.set('X-GitHub-Delivery', 'delivery-uuid-1'); + req.headers.contentType = ContentType.json; + req.write(jsonEncode({'action': 'opened'})); + var res = await req.close(); + expect(res.statusCode, HttpStatus.ok); + + // Replay of same delivery ID: rejected with 409 + req = await client.postUrl(Uri.parse('http://127.0.0.1:$port/hook')); + req.headers.set('X-GitHub-Event', 'issues'); + req.headers.set('X-GitHub-Delivery', 'delivery-uuid-1'); + req.headers.contentType = ContentType.json; + req.write(jsonEncode({'action': 'opened'})); + res = await req.close(); + expect(res.statusCode, HttpStatus.conflict); + + client.close(); + }); + }); +} diff --git a/test/unit/transport_test.dart b/test/unit/transport_test.dart new file mode 100644 index 00000000..ef71128f --- /dev/null +++ b/test/unit/transport_test.dart @@ -0,0 +1,445 @@ +import 'dart:convert'; +import 'package:github/github.dart'; +import 'package:http/http.dart' as http; +import 'package:http/testing.dart'; +import 'package:test/test.dart'; + +void main() { + group('HttpTransport URI and Query Resolution', () { + late GitHub github; + + setUp(() { + github = GitHub(); + }); + + tearDown(() { + github.dispose(); + }); + + test('resolves relative path against default endpoint', () { + final uri = github.transport.resolveUri('/user'); + expect(uri.toString(), equals('https://api.github.com/user')); + }); + + test('preserves absolute URLs', () { + final uri = github.transport + .resolveUri('https://uploads.github.com/repos/o/r/releases/1/assets'); + expect(uri.toString(), + equals('https://uploads.github.com/repos/o/r/releases/1/assets')); + }); + + test('expands standard and multi-segment path parameters', () { + final uri = github.transport.resolveUri( + '/repos/{owner}/{repo}/contents/{+path}', + pathParams: { + 'owner': 'octocat', + 'repo': 'hello-world', + 'path': 'src/nested/file.txt', + }, + multiSegmentParams: {'path'}, + ); + expect( + uri.toString(), + equals( + 'https://api.github.com/repos/octocat/hello-world/contents/src/nested/file.txt'), + ); + }); + + test('encodes query parameters, skipping nulls and expanding lists', () { + final uri = github.transport.resolveUri( + '/repos/o/r/issues', + params: { + 'state': 'open', + 'labels': ['bug', 'help wanted'], + 'sort': null, + 'page': 2, + }, + ); + expect( + uri.toString(), + equals( + 'https://api.github.com/repos/o/r/issues?state=open&labels=bug&labels=help+wanted&page=2'), + ); + }); + + test('merges params with existing query without duplicate ?', () { + final uri = github.transport.resolveUri( + '/repos/o/r/issues?direction=asc', + params: {'page': 1}, + ); + expect( + uri.toString(), + equals('https://api.github.com/repos/o/r/issues?direction=asc&page=1'), + ); + }); + }); + + group('buildQueryString', () { + test('handles empty or all-null map', () { + expect(buildQueryString({}), equals('')); + expect(buildQueryString({'a': null, 'b': null}), equals('')); + }); + + test('properly encodes keys and values', () { + expect( + buildQueryString({'user name': 'john doe', 'q': 'a&b=c'}), + equals('?user+name=john+doe&q=a%26b%3Dc'), + ); + }); + + test('handles without leading question mark', () { + expect( + buildQueryString({'page': 1}, prefixQuestionMark: false), + equals('page=1'), + ); + }); + }); + + group('HttpTransport Security Boundaries', () { + test('disallows insecure HTTP auth when allowInsecureAuth is false', + () async { + final client = MockClient((request) async => http.Response('{}', 200)); + final gh = GitHub( + auth: const Authentication.withToken('secret-token'), + endpoint: 'http://insecure-api.local', + client: client, + allowInsecureAuth: false, + ); + + expect( + () => gh.transport.execute( + const ApiRequest(method: 'GET', path: '/user'), + ), + throwsA(isA().having( + (e) => e.message, + 'message', + contains('Insecure HTTP authentication is disallowed'), + )), + ); + gh.dispose(); + }); + + test('allows insecure HTTP auth when explicitly enabled', () async { + final client = MockClient((request) async { + expect(request.headers['Authorization'], equals('token secret-token')); + return http.Response('{"login":"test"}', 200, + headers: {'content-type': 'application/json'}); + }); + final gh = GitHub( + auth: const Authentication.withToken('secret-token'), + endpoint: 'http://insecure-api.local', + client: client, + allowInsecureAuth: true, + ); + + final res = await gh.transport.execute( + const ApiRequest(method: 'GET', path: '/user'), + ); + expect(res.statusCode, equals(200)); + gh.dispose(); + }); + + test('strips authorization header when requesting untrusted origins', + () async { + String? sentAuth; + final client = MockClient((request) async { + sentAuth = request.headers['Authorization']; + return http.Response('{}', 200); + }); + final gh = GitHub( + auth: const Authentication.withToken('secret-token'), + client: client, + ); + + await gh.transport.execute( + const ApiRequest( + method: 'GET', path: 'https://attacker.com/leak-token'), + ); + expect(sentAuth, isNull); + gh.dispose(); + }); + }); + + group('RetryPolicy and Retries', () { + test('retries idempotent requests on 502/503/504 up to maxRetries', + () async { + var callCount = 0; + final client = MockClient((request) async { + callCount++; + if (callCount < 3) { + return http.Response('Bad Gateway', 502); + } + return http.Response('{"ok":true}', 200, + headers: {'content-type': 'application/json'}); + }); + + final delays = []; + final gh = GitHub( + client: client, + retryPolicy: RetryPolicy( + maxRetries: 3, + initialDelay: const Duration(milliseconds: 10), + sleeper: (d) async => delays.add(d), + random: () => 0.5, + ), + ); + + final res = await gh.transport.execute( + const ApiRequest(method: 'GET', path: '/repos/o/r'), + ); + expect(res.statusCode, equals(200)); + expect(callCount, equals(3)); + expect(delays.length, equals(2)); + gh.dispose(); + }); + + test('does NOT retry non-idempotent POST requests on 502', () async { + var callCount = 0; + final client = MockClient((request) async { + callCount++; + return http.Response('Bad Gateway', 502); + }); + + final gh = GitHub( + client: client, + retryPolicy: const RetryPolicy(maxRetries: 3), + ); + + await expectLater( + () => gh.transport.execute( + const ApiRequest(method: 'POST', path: '/repos/o/r/issues'), + ), + throwsA(isA()), + ); + expect(callCount, equals(1)); + gh.dispose(); + }); + + test('honors Retry-After integer header', () { + const policy = RetryPolicy(); + final res = http.Response('Too many requests', 429, headers: { + 'retry-after': '5', + }); + final delay = policy.delayFor(attempt: 0, response: res); + expect(delay, equals(const Duration(seconds: 5))); + }); + + test('honors Retry-After HTTP-Date header', () { + final now = DateTime.utc(2026, 9, 15, 12, 0, 0); + final policy = RetryPolicy(clock: () => now); + final res = http.Response('Too many requests', 429, headers: { + 'retry-after': 'Tue, 15 Sep 2026 12:00:10 GMT', + }); + final delay = policy.delayFor(attempt: 0, response: res); + expect(delay, equals(const Duration(seconds: 10))); + }); + }); + + group('ErrorDecoder and Redaction', () { + late GitHub github; + + setUp(() { + github = GitHub(); + }); + + tearDown(() { + github.dispose(); + }); + + test('redacts classic tokens and fine-grained PATs in messages and body', + () { + const raw = + 'Error with token ghp_ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789 and pat github_pat_11AAAAAAA0123456789abcdef_0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789abcdef'; + final redacted = ErrorDecoder.redact(raw); + expect(redacted, isNot(contains('ghp_'))); + expect(redacted, isNot(contains('github_pat_'))); + expect(redacted, contains('[REDACTED_TOKEN]')); + }); + + test('redacts Bearer authorization header values', () { + const raw = + 'Failed request with Authorization: Bearer secret_bearer_token'; + final redacted = ErrorDecoder.redact(raw); + expect(redacted, contains('Bearer [REDACTED]')); + expect(redacted, isNot(contains('secret_bearer_token'))); + }); + + test('safely truncates long bodies over 2000 characters', () { + final longBody = 'A' * 3000; + final sanitized = ErrorDecoder.sanitizeBody(longBody); + expect(sanitized.length, lessThan(2100)); + expect(sanitized, contains('... [TRUNCATED]')); + }); + + test('decodes 401 as NotAuthenticated', () { + final res = http.Response( + jsonEncode({'message': 'Bad credentials'}), + 401, + headers: {'content-type': 'application/json'}, + ); + expect( + () => ErrorDecoder.decode(github, res), + throwsA(isA() + .having((e) => e.statusCode, 'statusCode', 401) + .having((e) => e.message, 'message', contains('Bad credentials'))), + ); + }); + + test('decodes 403 secondary rate limit as RateLimitHit', () { + final res = http.Response( + jsonEncode({ + 'message': + 'You have exceeded a secondary rate limit. Please wait a few minutes.', + }), + 403, + headers: { + 'content-type': 'application/json', + 'x-ratelimit-remaining': '0', + 'x-ratelimit-reset': '1700000000', + }, + ); + expect( + () => ErrorDecoder.decode(github, res), + throwsA(isA() + .having((e) => e.remaining, 'remaining', 0) + .having((e) => e.reset, 'reset', isNotNull)), + ); + }); + + test('decodes 403 standard as AccessForbidden', () { + final res = http.Response( + jsonEncode({'message': 'Resource not accessible by integration'}), + 403, + headers: {'content-type': 'application/json'}, + ); + expect( + () => ErrorDecoder.decode(github, res), + throwsA(isA()), + ); + }); + + test('decodes 404 as NotFound', () { + final res = http.Response( + jsonEncode({'message': 'Not Found'}), + 404, + headers: {'content-type': 'application/json'}, + ); + expect( + () => ErrorDecoder.decode(github, res), + throwsA(isA()), + ); + }); + + test('decodes 409 as Conflict', () { + final res = http.Response( + jsonEncode({'message': 'Merge conflict'}), + 409, + headers: {'content-type': 'application/json'}, + ); + expect( + () => ErrorDecoder.decode(github, res), + throwsA(isA()), + ); + }); + + test('decodes 422 as ValidationFailed with structured errors', () { + final res = http.Response( + jsonEncode({ + 'message': 'Validation Failed', + 'errors': [ + { + 'resource': 'Issue', + 'field': 'title', + 'code': 'missing_field', + } + ] + }), + 422, + headers: {'content-type': 'application/json'}, + ); + expect( + () => ErrorDecoder.decode(github, res), + throwsA(isA() + .having((e) => e.errors, 'errors', isNotEmpty) + .having((e) => e.message, 'message', contains('Field: title'))), + ); + }); + + test('decodes 503 as ServiceUnavailable', () { + final res = http.Response( + 'Service Unavailable', + 503, + headers: {'content-type': 'text/plain'}, + ); + expect( + () => ErrorDecoder.decode(github, res), + throwsA(isA()), + ); + }); + + test('safely handles missing content-type and HTML error bodies', () { + final res = http.Response( + '500 Internal Server Error', + 500, + ); + expect( + () => ErrorDecoder.decode(github, res), + throwsA(isA()), + ); + }); + }); + + group('GitHub Compatibility Wrappers', () { + test( + 'request with statusCode: null returns response without throwing on 404', + () async { + final client = MockClient((request) async { + return http.Response('Not Found', 404); + }); + final gh = GitHub(client: client); + final response = await gh.request('GET', '/user/following/nonexistent'); + expect(response.statusCode, equals(404)); + gh.dispose(); + }); + + test( + 'requestJson handles 204 No Content returning null without FormatException', + () async { + final client = MockClient((request) async { + return http.Response('', 204); + }); + final gh = GitHub(client: client); + final result = await gh.requestJson, dynamic>( + 'DELETE', '/repos/o/r'); + expect(result, isNull); + gh.dispose(); + }); + + test('getJSON and postJSON execute through transport', () async { + final client = MockClient((request) async { + if (request.method == 'GET') { + return http.Response('{"name":"repo"}', 200, + headers: {'content-type': 'application/json'}); + } else if (request.method == 'POST') { + return http.Response('{"created":true}', 201, + headers: {'content-type': 'application/json'}); + } + return http.Response('Not Found', 404); + }); + final gh = GitHub(client: client); + + final getResult = await gh.getJSON, String>( + '/repos/o/r', + convert: (m) => m['name'] as String); + expect(getResult, equals('repo')); + + final postResult = await gh.postJSON, bool>( + '/repos/o/r/issues', + statusCode: 201, + body: {'title': 'New Issue'}, + convert: (m) => m['created'] as bool); + expect(postResult, isTrue); + gh.dispose(); + }); + }); +}